-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
chore: Check for readme changes in each PR #11495
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks pretty straight forward.
After thinking about this post-meeting, I did want to ask, is this what we want? We do not ask users to update the config in the etc/* directory. If so, I do think we should have a checklist for contributors beyond "make CI happy," but of commands, they should run before pushing to CI that need to pass. Thoughts?
@powersj good point, making it as easy for the contributor as possible is important. I updated the pull request template and the contribution doc to mention running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect I think this eases my concerns!
Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip. 👍 This pull request doesn't change the Telegraf binary size 📦 Click here to get additional PR build artifactsArtifact URLs |
The out of sync README.md have been updated in a separate PR, so now the CI passes: https://app.circleci.com/pipelines/github/influxdata/telegraf/11617/workflows/8870f983-a288-460c-8c30-72e777f123b5/jobs/189902?invite=true#step-104-8 |
resolve: #11303
Fail the CI if there are changes after running
make docs
so that the originally PR author knows to runmake docs
.Currently the CI is expected to fail until this is merged: #11489
You can see the error message here: https://app.circleci.com/pipelines/github/influxdata/telegraf/11612/workflows/97cd696d-8cf6-4756-a127-5611fb25af79/jobs/189853