Skip to content

Stop using auto#7024

Merged
yarikoptic merged 5 commits intodatalad:maintfrom
jwodder:gh-7016
Sep 19, 2022
Merged

Stop using auto#7024
yarikoptic merged 5 commits intodatalad:maintfrom
jwodder:gh-7016

Conversation

@jwodder
Copy link
Copy Markdown
Contributor

@jwodder jwodder commented Sep 9, 2022

Closes #7016. See https://github.com/jwodder/pr-merge-release-test for a test of the workflow in action.

Closes #6856 , Closes #6918

TODOs:

@jwodder jwodder added semver-internal Changes only affect the internal API CHANGELOG-missing When a PR's description does not contain a changelog item, yet. labels Sep 9, 2022
@github-actions github-actions Bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Sep 9, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 9, 2022

Codecov Report

Base: 75.41% // Head: 76.21% // Increases project coverage by +0.79% 🎉

Coverage data is based on head (8b8446e) compared to base (9567368).
Patch has no changes to coverable lines.

❗ Current head 8b8446e differs from pull request most recent head 8d99113. Consider uploading reports for the commit 8d99113 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            maint    #7024      +/-   ##
==========================================
+ Coverage   75.41%   76.21%   +0.79%     
==========================================
  Files         354      354              
  Lines       58751    58750       -1     
  Branches     6613     6613              
==========================================
+ Hits        44310    44779     +469     
+ Misses      14427    13957     -470     
  Partials       14       14              
Impacted Files Coverage Δ
datalad/_version.py 45.68% <0.00%> (-0.28%) ⬇️
datalad/tests/test_utils.py 70.28% <0.00%> (+0.09%) ⬆️
datalad/tests/utils_pytest.py 90.12% <0.00%> (+0.09%) ⬆️
datalad/distributed/ora_remote.py 80.46% <0.00%> (+0.15%) ⬆️
datalad/local/tests/test_add_archive_content.py 74.16% <0.00%> (+0.27%) ⬆️
datalad/support/tests/test_sshrun.py 70.73% <0.00%> (+1.21%) ⬆️
datalad/cli/tests/test_main.py 63.49% <0.00%> (+1.37%) ⬆️
datalad/tests/utils.py 56.58% <0.00%> (+7.92%) ⬆️
datalad/tests/test_tests_utils.py 68.55% <0.00%> (+68.55%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic
Copy link
Copy Markdown
Member

since we need now to wait for scriv to allow to work only on the target latest release for github (judging from author speed of reply it would be done in some foreseable future so we don't need to implement it ourselves), would you be able to add a helper for released auto plugin functionality, i.e. to annotate issues and PRs with the version it was fixed in?

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 13, 2022

@yarikoptic Commenting-script added; you can see it in action in the latest PRs in the test repo linked above.

Comment thread tools/ci/release-comment.py
@yarikoptic
Copy link
Copy Markdown
Member

ok, now we just need to wait for @nedbat to improve scriv (nedbat/scriv#57) and then we would merge/test-drive in datalad. Then IMHO we should centralize all this goodness into a GitHub action so it could be reused by datalad and its extensions at minimum. I have initiated https://github.com/datalad/release-action repository for that and made you @jwodder an admin for it happen you want to move this goodness there right now. Note that we have https://github.com/datalad/datalad-action already which could provide some hints on how github action could be developed.

@yarikoptic
Copy link
Copy Markdown
Member

note: @jwodder, I have updated description to close more issue and become TODOs instead of shortcomings (already addressed)

@yarikoptic yarikoptic added the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Sep 15, 2022
@github-actions github-actions Bot removed the CHANGELOG-missing When a PR's description does not contain a changelog item, yet. label Sep 15, 2022
@yarikoptic
Copy link
Copy Markdown
Member

yarikoptic commented Sep 15, 2022

4 @jwodder and FTR, to mitigate the issue of changelog commit pushed from github actions I have added yarikoptic-gitmate user with Write permissions to this repo and added GITMATE_GITHUB_TOKEN secret so workflow could use that token instead of default one to push.

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 15, 2022

@yarikoptic Do you want workflows to run on the changelog-creation commit? I thought you were concerned about running them on snippet-creation commits for the add-changelog-snippet.yml workflow.

@yarikoptic
Copy link
Copy Markdown
Member

yarikoptic commented Sep 15, 2022

Would it not be safe to rerun add-changelog-snippet.yml workflow upon such a push because label would no longer be assigned and thus that workflow would just do nothing (no infinite recursion of commits to happen)?

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 15, 2022

@yarikoptic The workflow won't run again once the "CHANGELOG-missing" label is removed, but there's a slight chance that, after the snippet is pushed, a new workflow run might start before the first workflow removes the label.

@yarikoptic
Copy link
Copy Markdown
Member

good observation! My thinking is: we can then removed label before pushing the snippet. But then if snippet push fails - we would end up without the snippet and label, which would be odd... but probably it would be also as odd if now push of snippet fails and we do not have label removed. So IMHO it would be ok to just swap to steps in the order so we would completely avoid such a race condition. Am I missing any potential drawback/side-effect @jwodder?

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 15, 2022

@yarikoptic I can't think of any drawbacks. Should I update add-changelog-snippet.yml in this PR or a separate one?

@yarikoptic
Copy link
Copy Markdown
Member

@yarikoptic I can't think of any drawbacks. Should I update add-changelog-snippet.yml in this PR or a separate one?

I think you can do right in this one and we will merge it altogether to give it a shot.

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 16, 2022

@yarikoptic Steps swapped. Do you also want GITMATE_GITHUB_TOKEN to be used in either or both workflows in this PR?

@yarikoptic
Copy link
Copy Markdown
Member

@yarikoptic Steps swapped.

Thank you!

Do you also want GITMATE_GITHUB_TOKEN to be used in either or both workflows in this PR?

I don't care much but may be let's do for both for consistency?

@jwodder
Copy link
Copy Markdown
Contributor Author

jwodder commented Sep 19, 2022

@yarikoptic Done: a73411a.

@yarikoptic
Copy link
Copy Markdown
Member

Thank you @jwodder ! So I will proceed with merge and we will try it out

@yarikoptic
Copy link
Copy Markdown
Member

FWIW new scriv https://pypi.org/project/scriv/0.17.0/ was released hours back and the change is

The github_release command now only considers the top-most entry in the changelog. You can use the --all option to continue the old behavior of making or updating GitHub releases for all of the entries.

so I do not think we need any tune up to this PR.

@yarikoptic yarikoptic merged commit 58afe90 into datalad:maint Sep 19, 2022
yarikoptic added a commit to yarikoptic/datalad that referenced this pull request Sep 19, 2022
…E_ one

after datalad#7024
we got release workflow erroring out with

	+ python3 tools/ci/release-comment.py datalad datalad 0.17.6 changelog.d/pr-6997.md changelog.d/pr-7009.md changelog.d/pr-7011.md changelog.d/pr-7024.md changelog.d/pr-7028.md changelog.d/pr-7036.md changelog.d/pr-7037.md changelog.d/pr-7039.md changelog.d/pr-7041.md
	Traceback (most recent call last):
	  File "/home/runner/work/datalad/datalad/tools/ci/release-comment.py", line 160, in <module>
		main()
	  File "/home/runner/work/datalad/datalad/tools/ci/release-comment.py", line 148, in main
		rc.comment_on_pr(prnum)
	  File "/home/runner/work/datalad/datalad/tools/ci/release-comment.py", line 108, in comment_on_pr
		self.comment_on_issueoid(
	  File "/home/runner/work/datalad/datalad/tools/ci/release-comment.py", line 123, in comment_on_issueoid
		r.raise_for_status()
	  File "/opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/site-packages/requests/models.py", line 1021, in raise_for_status
		raise HTTPError(http_error_msg, response=self)
	requests.exceptions.HTTPError: [40](https://github.com/datalad/datalad/actions/runs/3084869492/jobs/4987517315#step:7:41)1 Client Error: Unauthorized for url: https://api.github.com/repos/datalad/datalad/issues/6997/comments
	Error: Process completed with exit code 1.

so may be it was not the best
[decision](datalad#7024 (comment))
to use dedicated (but likely more limited in scopes) yarikoptic-gitmate token.
I think it would be good to just revert to use that generic token instead of
trying to figure out the scope and assigning a "better" gitmate one.
@yarikoptic-gitmate
Copy link
Copy Markdown
Collaborator

PR released in 0.17.6

@jwodder jwodder deleted the gh-7016 branch May 12, 2023 19:37
@jwodder jwodder added the release automation Automatic release and changelog generation label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release automation Automatic release and changelog generation semver-internal Changes only affect the internal API

Projects

None yet

3 participants