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.

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

docs: update docs to clarify path to documentation for previous versions of the validator #905

Merged
merged 19 commits into from
Aug 25, 2021

Conversation

lionel-nj
Copy link
Contributor

@lionel-nj lionel-nj commented Jun 15, 2021

Summary:

This PR clarifies the paths to documentation for previous versions of the validator
Expected behavior:

No code change.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

@lionel-nj lionel-nj self-assigned this Jun 15, 2021
@lionel-nj lionel-nj linked an issue Jun 15, 2021 that may be closed by this pull request
Copy link
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

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

IMHO this is still a bit confusing. For example, if I were new to the repo I'd think "There is a v2.0.0 in the list, but the below README also says v2.0.0 in the documentation, so they're probably the same".

I think there needs to be some explanation of what this README is, in context of the previous releases.

Maybe something like:

This README contains information for master branch of this project, which is under active development. If you'd like to view documentation for past releases of the project, see:

@lionel-nj
Copy link
Contributor Author

Thanks for the clarification @barbeau.

@lionel-nj lionel-nj requested a review from barbeau June 15, 2021 19:51
README.md Show resolved Hide resolved
@lionel-nj lionel-nj requested a review from barbeau June 16, 2021 15:48
Copy link
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

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

Thanks @lionel-nj. More comments below.

I'd strongly suggest you have someone that's not familiar with the steps walk through the docs for each version (1.4.0, 2.0.0, and current master branch) after the next set of edits and try to complete the steps to make sure everything is accurate and understandable.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lionel-nj lionel-nj changed the title docs: clarify path to documentation for previous versions of the validator docs: update docs to clarify path to documentation for previous versions of the validator Jun 17, 2021
@lionel-nj lionel-nj linked an issue Jun 17, 2021 that may be closed by this pull request
@lionel-nj
Copy link
Contributor Author

lionel-nj commented Jun 17, 2021

@isabelle-dr while working on documentation update, I included suggestions to fix #912, PTAL at e776d1b .

@barbeau
Copy link
Member

barbeau commented Jun 17, 2021

@isabelle-dr while working on documentation update, I included suggestions to fix #912, PTAL at e776d1b .

I'd suggest nesting the 3 categories so it's clear they are all possible future rules:

@isabelle-dr
Copy link
Contributor

@lionel-nj just one thing to change here so the title "Common cases of bad data" matches the label we chose "Community rule".
After this, I think we're good to merge this PR.

@lionel-nj
Copy link
Contributor Author

@lionel-nj just one thing to change here so the title "Common cases of bad data" matches the label we chose "Community rule".
After this, I think we're good to merge this PR.

Fixed in da7a817. Merging to master.

@lionel-nj lionel-nj merged commit a912d89 into master Aug 25, 2021
@lionel-nj lionel-nj deleted the task/update-docs branch August 25, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible Future Rules link in README.md broken docs: clarify the path to v1.4.0, and v2.0.0 documentation.
3 participants