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

Check if issue is mentioned in changelog. #457

Merged
merged 3 commits into from Feb 1, 2021

Conversation

Spacetown
Copy link
Member

@Spacetown Spacetown commented Jan 21, 2021

Test was mentioned in #424. The check can be deactivated by adding the text "[no changelog]" to a single line in the comment.

@Spacetown Spacetown marked this pull request as draft January 21, 2021 21:50
@codecov
Copy link

codecov bot commented Jan 21, 2021

Codecov Report

Merging #457 (b328b45) into master (7a1b38e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #457   +/-   ##
=======================================
  Coverage   95.49%   95.49%           
=======================================
  Files          20       20           
  Lines        2444     2444           
  Branches      420      420           
=======================================
  Hits         2334     2334           
  Misses         48       48           
  Partials       62       62           
Flag Coverage Δ
ubuntu-18.04 95.00% <ø> (ø)
windows-2019 95.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a1b38e...b328b45. Read the comment docs.

@Spacetown Spacetown force-pushed the add_changelog_checker branch 13 times, most recently from bcb7494 to b7db0b8 Compare January 22, 2021 19:49
@Spacetown Spacetown marked this pull request as ready for review January 22, 2021 19:49
@Spacetown Spacetown added needs review Docs related to the documentation or website QA related to testing, build infrastructure, etc labels Jan 22, 2021
@Spacetown Spacetown requested a review from chrta January 28, 2021 19:27
@Spacetown Spacetown added this to the 4.3 milestone Jan 28, 2021
@chrta
Copy link
Contributor

chrta commented Jan 29, 2021

Code looks good. I have 2 questions:

  1. Should the check be mentioned in https://gcovr.com/en/stable/contributing.html#how-to-submit-a-pull-request? Also the fact that adding [no changelog] can be added there for minor changes.
  2. Do i understand correctly that adding [no changelog] to any commit message skips the check? So it is not necessary to add this to all commits in a pr, only to one commit.

@latk
Copy link
Member

latk commented Jan 29, 2021

As an alternative to looking at commit messages, we could consider looking at the pull request comment via the Github API. This would likely mean that the check would be performed only in the Action, not in the release checklist. In a shell script, the PR comment can be extracted like:

curl https://api.github.com/repos/gcovr/gcovr/pulls/457 | jq -r .body

However, I have no strong preference regarding PR comment vs commit message as the source for these tags. My concern with commit messages would be that a PR with multiple commits could include both commits with changes that don't have to be mentioned in the Changelog, and other commits with changes that should be mentioned nevertheless. A PR message would be easier to change afterwards during review. The advantage of commit messages is that they can be checked locally and without network connectivity.

But I do like that this PR introduces more structure into the CI pipelines!

@Spacetown
Copy link
Member Author

Spacetown commented Jan 29, 2021

  1. Should the check be mentioned in https://gcovr.com/en/stable/contributing.html#how-to-submit-a-pull-request? Also the fact that adding [no changelog] can be added there for minor changes.

Of course.

  1. Do i understand correctly that adding [no changelog] to any commit message skips the check? So it is not necessary to add this to all commits in a pr, only to one commit.

This was the intention but I find this idea better:

As an alternative to looking at commit messages, we could consider looking at the pull request comment via the Github API. This would likely mean that the check would be performed only in the Action, not in the release checklist.

Instead of using curl I'll try to use the available PR info in the pipeline and directly write the script there. I think the local check isn't realy needed because it is only needed for PR's and not for playing in the local clone.
With every approach we have the problem that the PR number is realy known after creating the PR.

@Spacetown Spacetown force-pushed the add_changelog_checker branch 2 times, most recently from cbe856f to e4caa4d Compare January 29, 2021 21:08
@Spacetown Spacetown force-pushed the add_changelog_checker branch 3 times, most recently from 31bc5f0 to fd262e5 Compare January 29, 2021 21:18
.github/workflows/test.yml Outdated Show resolved Hide resolved
@Spacetown Spacetown force-pushed the add_changelog_checker branch 2 times, most recently from 1c42eb5 to ff90a32 Compare January 29, 2021 22:27
Copy link
Member

@latk latk left a comment

Choose a reason for hiding this comment

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

LGTM :)

Thanks for implementing this! Automating such checks will help future releases to be ready more quickly.

@Spacetown Spacetown merged commit ba45795 into gcovr:master Feb 1, 2021
@Spacetown Spacetown deleted the add_changelog_checker branch February 1, 2021 09:00
@Spacetown Spacetown restored the add_changelog_checker branch February 1, 2021 09:24
@Spacetown Spacetown deleted the add_changelog_checker branch February 1, 2021 09:24
@Spacetown
Copy link
Member Author

@chrta @latk: For the main branch the test is turned of. I'll create a new PR in the afternoon.

@gobater
Copy link
Contributor

gobater commented Feb 1, 2021

@Spacetown : please update documentation on this topic in https://gcovr.com/en/stable/contributing.html#how-to-submit-a-pull-request

@Spacetown
Copy link
Member Author

@Spacetown : please update documentation on this topic in https://gcovr.com/en/stable/contributing.html#how-to-submit-a-pull-request

That's the documentation of the stable version. Please take a look at the latest source https://github.com/gcovr/gcovr/blob/master/CONTRIBUTING.rst#how-to-submit-a-pull-request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs related to the documentation or website needs review QA related to testing, build infrastructure, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants