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

Document current acl:trustedApp system #56

Merged
merged 3 commits into from
Jun 17, 2019
Merged

Document current acl:trustedApp system #56

merged 3 commits into from
Jun 17, 2019

Conversation

michielbdejong
Copy link
Contributor

Fixes #38

@michielbdejong michielbdejong mentioned this pull request May 28, 2019
Closed
@michielbdejong
Copy link
Contributor Author

@kjetilk please review? I added the note that it was only added recently and is likely to change in the near future.

@michielbdejong michielbdejong mentioned this pull request May 29, 2019
Open
kjetilk
kjetilk previously approved these changes May 29, 2019
View reviewed changes
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

LGTM

@michielbdejong
Copy link
Contributor Author

Thanks! Now it looks like we need a second review by a reviewer with write access, and then someone should merge it.

Not sure who is currently a reviewer on this repo and could help with that - @RubenVerborgh and @dmitrizagidulin probably?

@Mitzi-Laszlo can you also add @jaxoncreed and me on this repo?

@michielbdejong
Copy link
Contributor Author

@megoth maybe?

RubenVerborgh
RubenVerborgh reviewed May 29, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
@michielbdejong michielbdejong mentioned this pull request May 31, 2019
Closed
@michielbdejong
Copy link
Contributor Author

@kjetilk @RubenVerborgh can you re-review with that latest edit? ^

kjetilk
kjetilk approved these changes Jun 17, 2019
View reviewed changes
Copy link
Member

@kjetilk kjetilk left a comment

Choose a reason for hiding this comment

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

LGTM

@kjetilk kjetilk requested a review from megoth June 17, 2019 10:31
jaxoncreed
jaxoncreed approved these changes Jun 17, 2019
View reviewed changes
Copy link

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

:shipit:

RubenVerborgh
RubenVerborgh approved these changes Jun 17, 2019
View reviewed changes
Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

No immediate concerns.

@kjetilk kjetilk merged commit a71580b into solid:master Jun 17, 2019
@TallTed
Copy link
Contributor

TallTed commented Jun 17, 2019

"added recently" is ambiguous -- and will surprisingly quickly become inaccurate. I strongly advise including some absolute date detail.

@elf-pavlik elf-pavlik mentioned this pull request Apr 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kjetilk kjetilk kjetilk approved these changes

@RubenVerborgh RubenVerborgh RubenVerborgh approved these changes

@jaxoncreed jaxoncreed jaxoncreed approved these changes

@megoth megoth Awaiting requested review from megoth

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

remove "possible future" label from acl:trustedApp paragraph
5 participants
@michielbdejong @TallTed @kjetilk @RubenVerborgh @jaxoncreed