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

infra: Provide custom prefix to auto-related labels #6151

Merged
merged 1 commit into from Nov 11, 2021

Conversation

adswa
Copy link
Member

@adswa adswa commented Nov 8, 2021

In looking through the issues closed over the weekend, I felt compelled to act on the suggestion in #5833 to prefix a common, versioning-related identifier to the PR labels for the auto-workflow.
I think there was some general agreement in the original issue, but not enough momentum to implement the change. I personally keep forgetting which labels exist, and would be in favor of having them in one group to ease discoverability.
I'm suggesting semver as a prefix, because the corresponding test is called "Test for semver label". I'm happy to change this into anything else, though.

  • if this change is accepted, the label names need to be changed

@adswa adswa added the semver-internal Changes only affect the internal API label Nov 8, 2021
@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #6151 (229dd5f) into master (0df06be) will decrease coverage by 0.50%.
The diff coverage is n/a.

❗ Current head 229dd5f differs from pull request most recent head 349a021. Consider uploading reports for the commit 349a021 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6151      +/-   ##
==========================================
- Coverage   90.21%   89.70%   -0.51%     
==========================================
  Files         312      318       +6     
  Lines       42235    41854     -381     
==========================================
- Hits        38104    37547     -557     
- Misses       4131     4307     +176     
Impacted Files Coverage Δ
datalad/_version.py 46.99% <0.00%> (-53.01%) ⬇️
datalad/support/due.py 48.00% <0.00%> (-32.00%) ⬇️
datalad/distribution/uninstall.py 71.42% <0.00%> (-28.58%) ⬇️
datalad/metadata/extractors/tests/test_exif.py 92.00% <0.00%> (-8.00%) ⬇️
datalad/metadata/extractors/tests/test_image.py 92.00% <0.00%> (-8.00%) ⬇️
datalad/interface/shell_completion.py 92.59% <0.00%> (-7.41%) ⬇️
datalad/distributed/create_sibling_gitlab.py 68.32% <0.00%> (-6.68%) ⬇️
datalad/metadata/extractors/tests/test_audio.py 93.54% <0.00%> (-6.46%) ⬇️
datalad/support/tests/test_due_utils.py 78.04% <0.00%> (-4.88%) ⬇️
datalad/interface/utils.py 91.36% <0.00%> (-3.96%) ⬇️
... and 85 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 0df06be...349a021. Read the comment docs.

@yarikoptic
Copy link
Member

I wonder on what disturbance that would cause interim since if we rename the labels and workflow in master -- maint from which we do cut releases wouldn't be able to auto release, so it would break that component.
If we are to do such a change, I feel it should happen right after some release so both maint and master could have a chance to auto-release without problems and collate perspective changelog based on labels of already merged PRs etc.

@mih
Copy link
Member

mih commented Nov 9, 2021

I am all for such a change. I am totally unaware of what disturbances it would cause. But I do know that a release from master didn't work already with 0.15.0 (so no breakage possible here), and trying right after a release will only delay a reliable test of whether there is really no breakage. The worth case scenario -- an auto release does not work -- is nothing that concerns me at all.

@adswa
Copy link
Member Author

adswa commented Nov 10, 2021

With @mih's statement and @bpoldrack's thumbs up this kinda sounds like we could proceed here? AFAIK we could rename the current labels by editing them

@yarikoptic
Copy link
Member

FWIW, on a 2nd thought, since auto analyzes PR labels only when running, and renaming (not creating new) hopefully would remain them in already labeled PRs, I think it should be safe to proceed but this PR most likely need to be done against maint, and then maint merged into master so everything is consistent and nice.

upon rename -- would be worthwhile to check recently merged PRs if their labels were updated correspondingly.

@mih
Copy link
Member

mih commented Nov 11, 2021

Sound like a plan! Thx!

This change is an attempt to act on the suggestion in datalad#5833 to ease
understanding in how PRs need to be labelled for the 'auto' versioning
workflow. The test checking for appropriate labelling of PRs is called
'semver', hence my initial suggestion here in this change is a semver
prefix in hopes to make the relevant issues more discoverable to
new contributors.
@adswa adswa changed the base branch from master to maint November 11, 2021 07:58
@adswa
Copy link
Member Author

adswa commented Nov 11, 2021

I rebased to maint, and will edit the labels now.

@adswa adswa added semver-internal Changes only affect the internal API and removed semver-internal Changes only affect the internal API labels Nov 11, 2021
@adswa
Copy link
Member Author

adswa commented Nov 11, 2021

I had to re-attach the label to the PR to have it picked up by the semver test

@mih
Copy link
Member

mih commented Nov 11, 2021

I killed the appveyor tests -- nothing in here is relevant for appveyor.

@mih
Copy link
Member

mih commented Nov 11, 2021

OK, let's do it.

@mih mih merged commit 2eecb34 into datalad:maint Nov 11, 2021
@adswa adswa deleted the mnt-5833 branch November 15, 2021 07:24
@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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants