scientific-python / attach-next-milestone-action

Attach the next open milestone to a merged PR
BSD 3-Clause "New" or "Revised" License
2 stars 3 forks source link

Example working deployment? #3

Closed pllim closed 1 year ago

pllim commented 1 year ago

I am interesting in this but I am surprised it does not need a Dockerfile and entrypoint shell script, since the Action was written in Python and not browser-native language like NodeJS. Is this being used anywhere? I wish to see how it is working in production. Thanks!

stefanv commented 1 year ago

It's been working fine for skimage: https://github.com/scikit-image/scikit-image/blob/main/.github/workflows/milestone-merged-prs.yaml

pllim commented 1 year ago

Interesting... I guess Python based Action support must have been improved since I last wrote one. Maybe as long as you stick to the packages that already come in "ubuntu-latest", you are okay? Is that also why you don't use PyGithub here?

stefanv commented 1 year ago

No, actually I should probably have used an existing library, but I didn't know which one was "official". The API call is straightforward, and here I was lucky because you'd typically need paging—but few projects have >30 open milestones.

pllim commented 1 year ago

but few projects have >30 open milestones

But one of them is bound to use this and open a bug report. 😆

Maybe such limitation should be documented, if not already.

stefanv commented 1 year ago

Better to improve the API fetcher. We can also set the limit to 100 for now. Show me the project with >100 open milestones!!

pllim commented 1 year ago

I have used both https://github3.readthedocs.io and https://pygithub.readthedocs.io . I somehow prefer API of the latter but otherwise I think they both work okay.

stefanv commented 1 year ago

Here is where requests comes from, btw: https://github.com/scientific-python/attach-next-milestone-action/blob/main/action.yaml#L27

pllim commented 1 year ago

Oh. This is so much simpler than how I set it up over at https://github.com/scientific-python/action-check-changelogfile . I almost wanna cry.

stefanv commented 1 year ago

The pygithub invocation of next milestone may be as simple as:

gh.get_repo('networkx/networkx').get_milestones(sort='date')[0].number
pllim commented 1 year ago

Well, the [0] subscript might or might not work. For example, astropy already has v6.1 milestone in https://github.com/astropy/astropy/milestones even though our next release is v6.0. This is because we opened issues to remind us to do stuff after the next feature release (e.g., removing deprecated stuff).

stefanv commented 1 year ago

Oh, right, actually I'm looking for the opposite... the oldest open milestone. But you get the idea.

stefanv commented 1 year ago

Current logic:

milestones = [m for m in milestones if is_version(m['title'])]
milestones = sorted(milestones, key=lambda x: Version(x['title']))
print(milestones[0]['number'])
stefanv commented 1 year ago
gh.get_repo('networkx/networkx').get_milestones(sort='date', direction='reverse')[0].number
stefanv commented 1 year ago

The current script relies on the fact that your milestone can be converted to a version. I also don't know if GitHub knows anything about version numbers, so is the sorting correct? :shrug:

I think NetworkX uses nx-version sometimes, so script would have to be modified to handle their use-case.

pllim commented 1 year ago

I don't think you want sort='date' because that appears to sort by milestone creation date. I think it is just a coincidence that some 5.x bugfix series are not listed on top of v6.0 here for astropy/astropy.

Milestone(title="v6.0", number=98)
Milestone(title="v5.0.7", number=108)
Milestone(title="v5.3.2", number=112)
Milestone(title="v6.1", number=111)
pllim commented 1 year ago

And nothing stops someone to also add bugfix number into the next feature milestone, like "v6.0.0".

Maybe there is no fool-proof way to make everyone happy, or maybe I am overthinking this.

bsipocz commented 1 year ago

I fully agree that the current way works very well for projects releasing from main, but not as fool-proof as one would like if there are bugfix branches with milestones, let alone if there are multiple of these at the same time. But I suppose we could move stuff around and have this eventually the one place where all the logic just works?