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

chore: Check for readme changes in each PR #11495

Merged
merged 5 commits into from
Jul 12, 2022
Merged

chore: Check for readme changes in each PR #11495

merged 5 commits into from
Jul 12, 2022

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Jul 12, 2022

resolve: #11303

Fail the CI if there are changes after running make docs so that the originally PR author knows to run make 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

@sspaink sspaink requested review from powersj and srebhan July 12, 2022 16:23
@telegraf-tiger telegraf-tiger bot added the chore label Jul 12, 2022
@sspaink sspaink added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jul 12, 2022
Copy link
Contributor

@powersj powersj left a 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?

@sspaink
Copy link
Contributor Author

sspaink commented Jul 12, 2022

@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 make docs. What do you think about that? Any other places we should mention it? I do think asking the contributor to run it shouldn't be too big of an ask, it is less work then updating the etc configs.

Copy link
Contributor

@powersj powersj left a 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!

@sspaink
Copy link
Contributor Author

sspaink commented Jul 12, 2022

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

@sspaink sspaink requested a review from MyaLongmire July 12, 2022 19:43
@sspaink sspaink merged commit 19b77ad into master Jul 12, 2022
@sspaink sspaink deleted the makedocs branch July 12, 2022 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Check for updated README for sample.conf changes
2 participants