Skip to content

Conversation

senden9
Copy link
Contributor

@senden9 senden9 commented Oct 10, 2019

Fixes #28.

See https://travis-ci.org/senden9/influxdb-rust/jobs/595985325 for a check that failed because of a non-matching readme.
See https://travis-ci.org/senden9/influxdb-rust/jobs/596020860 for a passed check because of a readme that is in sync.

Please check the 'Planned Features" because I simply merged the two different lists and subtracted the "Currently Supported Features".

Note: This commit should fail on CI because the readmes do not match.
Note: Use absolute links for CODE_OF_CONDUCT and CONTRIBUTING so that
we can click on this links also on docs.rs.
Copy link
Collaborator

@Empty2k12 Empty2k12 left a comment

Choose a reason for hiding this comment

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

Thank you for this PR 🎉, I like how it turned out. Just some minor comments.

Could there also be a better message when the diff fails? E.g. "README.md out of date. Run cargo readme > README.md and commit again.".

if [ $? -eq 0 ]
then
    echo "README.md is up to date!"
    exit 0
else
    echo "README.md out of date. Run `cargo readme > README.md` and commit again."
    exit 1
fi

@Empty2k12 Empty2k12 changed the title check readme automatically (fix #28) Switch to cargo-readme for README generation Oct 10, 2019
@Empty2k12
Copy link
Collaborator

Could we also add a sentence to the CONTRIBUTING.md file documenting the additional step of running cargo readme > README.md when the cargodoc comment in lib.rs changed?

Also move files into own folder.
Remove this test branch from the travis
@senden9 senden9 force-pushed the fix/28-cargo-readme branch from b4d57bf to 0d27c62 Compare October 10, 2019 10:17
@senden9
Copy link
Contributor Author

senden9 commented Oct 10, 2019

According to the CONTRIBUTING.md. I would insert if under "Pull Requests" but there is a problem with a dead link. Because of this I do not find the right point to insert this information. See https://github.com/Empty2k12/influxdb-rust/blame/4f1d4f6f4d98def7ec33aa9752f8b5db255b9857/CONTRIBUTING.md#L97

@Empty2k12
Copy link
Collaborator

Seems like I forgot to add a Pull Request Template indeed.

It should be added under .github/PULL_REQUEST_TEMPLATE.md:

## Description

{ describe your changes here }

### Checklist
- [ ] Formatted code using `cargo fmt --all`
- [ ] Linted code using clippy `cargo clippy --all-targets --all-features -- -D warnings`
- [ ] Updated README.md using `cargo readme > README.md`
- [ ] Reviewed the diff. Did you leave any print statements or unnecessary comments?
- [ ] Any unfinished work that warrants a separate issue captured in an issue with a TODO code comment

The broken link you found can be fixed by changing the file to ./.github/PULL_REQUEST_TEMPLATE.md

@Empty2k12
Copy link
Collaborator

@senden9
Do you want to add the PULL_REQUEST_TEMPLATE and rebase this PR, or should I so that?

@senden9
Copy link
Contributor Author

senden9 commented Oct 11, 2019

I do not know where PULL_REQUEST_TEMPLATE is (if it exists already). Searched in the master branch. So I think it would be easier if you do it :)

@Empty2k12 Empty2k12 self-requested a review October 11, 2019 18:22
Copy link
Collaborator

@Empty2k12 Empty2k12 left a comment

Choose a reason for hiding this comment

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

@senden9 Thank you for creating this PR! 🎉It will be very helpful in keeping code and README up-to-date!

I wish you continued success with Hacktoberfest!

@Empty2k12 Empty2k12 added Status: Merge when CI passes This PR has been reviewed and accepted for merge and removed Status: Pending Updates labels Oct 11, 2019
@Empty2k12 Empty2k12 merged commit 0f615aa into influxdb-rs:master Oct 11, 2019
@senden9
Copy link
Contributor Author

senden9 commented Oct 13, 2019

cargo-readme, is a useful tool, so actually thanks for opening that issue so I could learn a bit about it :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate cargo-readme to keep all documentation up-to-date
2 participants