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

docs: explain specific nixos configurations #11029

Merged
merged 1 commit into from
Oct 4, 2022
Merged

docs: explain specific nixos configurations #11029

merged 1 commit into from
Oct 4, 2022

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Sep 21, 2022

Description of your changes:
Explain the fix for #10932 to let NixOS users set up Rook more easily.

Which issue is resolved by this Pull Request:
Resolves #10942 (comment)

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide).
  • Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR. I can't!
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@moduon MT-1248

@parth-gr
Copy link
Member

PLease add a signed-off in the commit message

parth-gr
parth-gr reviewed Sep 21, 2022
View reviewed changes
Documentation/Getting-Started/Prerequisites/prerequisites.md Outdated

If you install Rook with Helm, add this to your `values.yaml`:

```yaml
Copy link
Member

Choose a reason for hiding this comment

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

@travisn how about also leaving this setting commented in the manifest files?

Copy link
Member

Choose a reason for hiding this comment

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

per comment above I agree it would be preferred to keep this doc simpler.

@yajo
Copy link
Contributor Author

yajo commented Sep 26, 2022

Signed.

@parth-gr
Copy link
Member

DCO still fails, can you please fix that,

It shows the error
Commit sha: [d074f75](https://github.com/rook/rook/pull/11029/commits/d074f754c9fefd04c14fab6a4c50ea36fc29c442), Author: Jairo Llopis, Committer: Jairo Llopis; Expected "Jairo Llopis [973709+yajo@users.noreply.github.com](mailto:973709+yajo@users.noreply.github.com)", but got "Jairo Llopis [jairo@moduon.team](mailto:jairo@moduon.team)".

@parth-gr
Copy link
Member

DCO still fails, can you please fix that,

It shows the error Commit sha: [d074f75](https://github.com/rook/rook/pull/11029/commits/d074f754c9fefd04c14fab6a4c50ea36fc29c442), Author: Jairo Llopis, Committer: Jairo Llopis; Expected "Jairo Llopis [973709+yajo@users.noreply.github.com](mailto:973709+yajo@users.noreply.github.com)", but got "Jairo Llopis [jairo@moduon.team](mailto:jairo@moduon.team)".

Seems like not a correct sign-off

@yajo
Copy link
Contributor Author

yajo commented Sep 26, 2022

Let's see now.

parth-gr
parth-gr approved these changes Sep 26, 2022
View reviewed changes
Copy link
Member

@parth-gr parth-gr left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Let @travisn review it once.

travisn
travisn requested changes Sep 27, 2022
View reviewed changes
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

For the commit message, let's just have a "docs:" prefix, something like this:

docs: explain specific nixos configurations

Documentation/Getting-Started/Prerequisites/prerequisites.md Outdated Show resolved Hide resolved
Documentation/Getting-Started/Prerequisites/prerequisites.md Outdated Show resolved Hide resolved
Documentation/Getting-Started/Prerequisites/prerequisites.md Outdated

If you install Rook with Helm, add this to your `values.yaml`:

```yaml
Copy link
Member

Choose a reason for hiding this comment

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

per comment above I agree it would be preferred to keep this doc simpler.

@yajo
Copy link
Contributor Author

yajo commented Sep 28, 2022

All attended.

@travisn
Copy link
Member

travisn commented Sep 28, 2022

For the commit message, let's just have a "docs:" prefix, something like this:

docs: explain specific nixos configurations

Just one more nit on the commit message, thanks!

@travisn travisn changed the title docs(nixos): explain specific configurations docs: explain specific nixos configurations Sep 28, 2022
@yajo
docs: explain specific nixos configurations
7fecca3 
Explain the fix for rook#10932 to let NixOS users set up Rook more easily.

Signed-off-by: Jairo Llopis <jairo@moduon.team>
@yajo
Copy link
Contributor Author

yajo commented Oct 4, 2022

You got it!

@travisn travisn merged commit 9e89c8e into rook:master Oct 4, 2022
@mergify mergify bot mentioned this pull request Oct 4, 2022
Merged
mergify bot added a commit that referenced this pull request Oct 4, 2022
@mergify
Merge pull request #11098 from rook/mergify/bp/release-1.10/pr-11029
20f227b 
docs: explain specific nixos configurations (backport #11029)
@yajo yajo deleted the docs-nixos branch October 5, 2022 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@travisn travisn travisn approved these changes

@parth-gr parth-gr parth-gr approved these changes

@Madhu-1 Madhu-1 Awaiting requested review from Madhu-1

Assignees
No one assigned
Labels
backport-release-1.10 docs skip-ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yajo @parth-gr @travisn @subhamkrai