Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reva/frontend: Add capabilities to indicate attributes that are read-only #6047

Merged
merged 5 commits into from
Apr 19, 2023
Merged

reva/frontend: Add capabilities to indicate attributes that are read-only #6047

merged 5 commits into from
Apr 19, 2023

Conversation

Excds
Copy link

@Excds Excds commented Apr 13, 2023 •
edited
Loading

This adds capabilities indicating if certain attributes are read-only.

Companion PR: cs3org/reva#3789

@Excds Excds added the Status:Needs-Review Needs review from a maintainer label Apr 13, 2023
@Excds Excds requested review from rhafer and kobergj April 13, 2023 10:25
@Excds Excds self-assigned this Apr 13, 2023
@update-docs
Copy link

update-docs bot commented Apr 13, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@Excds Excds mentioned this pull request Apr 13, 2023
Merged
@mmattel
Copy link
Contributor

mmattel commented Apr 13, 2023

Please add some text to readme.md to reflect the changes on the envvar side.
I can help finetuning after a first writeup.

@Excds Excds force-pushed the excds/feature/add_capabilities_indicating_read-only_attributes branch from 4e9c413 to f5b2d98 Compare April 14, 2023 08:45
mmattel
mmattel approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@mmattel mmattel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a docs pov, but needs a dev reviewer approval too

@micbar
Copy link
Contributor

micbar commented Apr 14, 2023

@mmattel this has changed in the reva implementation. We will have only one config value and a list of strings.

@Excds
Copy link
Author

Excds commented Apr 14, 2023

@mmattel this has changed in the reva implementation. We will have only one config value and a list of strings.

@micbar In the ocis implementation, each attribute is still an environment variable.

@micbar
Copy link
Contributor

micbar commented Apr 14, 2023

@mmattel this has changed in the reva implementation. We will have only one config value and a list of strings.

@micbar In the ocis implementation, each attribute is still an environment variable.

That surprises me, it would mean we need a code change for every new attribute.

Daniel Swärd and others added 2 commits April 17, 2023 10:07
@Excds Excds force-pushed the excds/feature/add_capabilities_indicating_read-only_attributes branch from de6c4bc to e52a507 Compare April 17, 2023 08:39
@Excds Excds force-pushed the excds/feature/add_capabilities_indicating_read-only_attributes branch from e52a507 to 9a2ecf4 Compare April 17, 2023 09:02
@Excds Excds force-pushed the excds/feature/add_capabilities_indicating_read-only_attributes branch from 9a2ecf4 to 1938495 Compare April 17, 2023 09:10
kobergj
kobergj requested changes Apr 17, 2023
View reviewed changes
services/frontend/pkg/config/config.go Outdated Show resolved Hide resolved
@micbar
Copy link
Contributor

micbar commented Apr 17, 2023

@Excds Can we make the adjustments and merge this today to unblock the frontend team?

@micbar
Copy link
Contributor

micbar commented Apr 17, 2023

AFAICT there is no reva update inside this PR, this will be needed to make the changes work.

kobergj
kobergj requested changes Apr 18, 2023
View reviewed changes
Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more question

services/frontend/pkg/revaconfig/config.go Show resolved Hide resolved
@mmattel
Apply suggestions from code review
78d7381 
Co-authored-by: Martin <github@diemattels.at>
@sonarcloud
Copy link

sonarcloud bot commented Apr 19, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@micbar micbar merged commit dd8e752 into master Apr 19, 2023
@delete-merged-branch delete-merged-branch bot deleted the excds/feature/add_capabilities_indicating_read-only_attributes branch April 19, 2023 08:33
ownclouders pushed a commit that referenced this pull request Apr 19, 2023
@micbar
Merge pull request #6047 from owncloud/excds/feature/add_capabilities…
c9f7904 
…_indicating_read-only_attributes

reva/frontend: Add capabilities to indicate attributes that are read-only
@mmattel mmattel mentioned this pull request Apr 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann left review comments

@mmattel mmattel mmattel approved these changes

@kobergj kobergj kobergj approved these changes

@rhafer rhafer rhafer approved these changes

Assignees

@Excds Excds

Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Excds @mmattel @micbar @rhafer @kulmann @kobergj