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

Use version suffixes in PR title instead of Python versions when using --new-pr #4253

Merged
merged 4 commits into from Jun 23, 2023

Conversation

Micket
Copy link
Contributor

@Micket Micket commented May 4, 2023

Has bothered me for a while now. the w/ Python 3.x.y in the titles are pretty much useless these days, and instead important information like if a CUDA build is left out.
This puts all the suffixes in there instead.

@Micket Micket marked this pull request as draft May 4, 2023 15:41
@Micket Micket changed the title Use version suffixes in title instead of Python versions Use version suffixes in PR title instead of Python versions when using --new-pr May 4, 2023
@boegel boegel added the change label May 10, 2023
@boegel boegel added this to the 4.x milestone May 10, 2023
@boegel
Copy link
Member

boegel commented May 10, 2023

@Micket Can you clarify why this is marked as draft?

@Micket
Copy link
Contributor Author

Micket commented May 10, 2023

I never got around to test any of the code at all. Might work, 100% untested

@Micket Micket marked this pull request as ready for review May 29, 2023 13:48
@Micket
Copy link
Contributor Author

Micket commented May 29, 2023

I think this works fine

$ eb --new-pr GATK-4.1.3.0-GCCcore-8.2.0-Java-1.8.eb -D  testing-1.2.3-foss-2022a-CUDA-11.7.0.eb 
== Temporary log file in case of crash /dev/shm/eb-5xcxca2p/easybuild-ugshj3o6.log
== fetching branch 'develop' from https://github.com/easybuilders/easybuild-easyconfigs.git...
== copying files to /dev/shm/eb-5xcxca2p/git-working-diryxxzy5g1/easybuild-easyconfigs...
== pushing branch '20230529154625_new_pr_GATK4130' to remote 'github_Micket_zMHdd' (git@github.com:Micket/easybuild-easyconfigs.git) [DRY RUN]

Opening pull request [DRY RUN]
* target: easybuilders/easybuild-easyconfigs:develop
* from: Micket/easybuild-easyconfigs:20230529154625_new_pr_GATK4130
* title: "{bio,devel}[GCCcore/8.2.0,foss/2022a] GATK v4.1.3.0, testing v1.2.3 w/ CUDA 11.7.0, Java 1.8"
* labels: new, update
* description:
"""

when testing with a few random easyconfigs

@branfosj
Copy link
Member

branfosj commented Jun 8, 2023

So the test for this whole new PR function is at

def test_github_new_pr_from_branch(self):
which neither tests multiple easyconfigs or anything with a versionsuffix.

I do think we should pull this code out into a separate function and test it separately. Then, in an outside the scope of this PR, we'd be able to hook it into the review PR code to have it suggest a PR title should be.

@Micket
Copy link
Contributor Author

Micket commented Jun 15, 2023

I'm not sure how to best set up tests for this. It's isolated to just the ecs variable but still quite a hassle to fake those.

@branfosj
Copy link
Member

I'm not sure how to best set up tests for this. It's isolated to just the ecs variable but still quite a hassle to fake those.

See Micket#2

@branfosj
Copy link
Member

Also, if there are any other unusual software names / versions / versionsuffixes we should test then let me know and I'll add some more.

@branfosj branfosj modified the milestones: 4.x, next release (4.7.3?) Jun 23, 2023
@branfosj branfosj merged commit f42ea41 into easybuilders:develop Jun 23, 2023
35 checks passed
@branfosj branfosj mentioned this pull request Jun 23, 2023
@Micket Micket deleted the title_suffix branch March 19, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants