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

doc: Add shouldExist for users #1080

Merged
merged 1 commit into from
Sep 8, 2020
Merged

doc: Add shouldExist for users #1080

merged 1 commit into from
Sep 8, 2020

Conversation

cgwalters
Copy link
Member

This addition to the docs was missed in the original PR
#1014

sohankunkerkar
sohankunkerkar reviewed Aug 29, 2020
View reviewed changes
Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

s/false/true

sohankunkerkar
sohankunkerkar approved these changes Aug 31, 2020
View reviewed changes
Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.
LGTM

arithx
arithx reviewed Aug 31, 2020
View reviewed changes
Copy link
Contributor

@arithx arithx left a comment

Choose a reason for hiding this comment

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

Looks like it's also missing for groups would you mind adding it there as well?

@sohankunkerkar
Copy link
Member

sohankunkerkar commented Aug 31, 2020 •
edited
Loading

Looks like it's also missing for groups would you mind adding it there as well?

Nice catch! I missed that part from the review.

@cgwalters
doc: Add shouldExist for users and groups
db3299a 
This addition to the docs was missed in the original PR
coreos#1014
@cgwalters
Copy link
Member Author

cgwalters commented Aug 31, 2020 •
edited
Loading

Looks like it's also missing for groups would you mind adding it there as well?

Done

@cgwalters cgwalters mentioned this pull request Sep 8, 2020
Closed
jlebon
jlebon approved these changes Sep 8, 2020
View reviewed changes
Copy link
Member

@jlebon jlebon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM; restarted CI on this (though it's a docs-only change, so feel free to merge too!).

@sohankunkerkar sohankunkerkar merged commit eecec7f into coreos:master Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arithx arithx arithx approved these changes

@jlebon jlebon jlebon approved these changes

@sohankunkerkar sohankunkerkar sohankunkerkar approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@cgwalters @sohankunkerkar @arithx @jlebon