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

Milestones, bugfixes, oh my. #3904

Closed
Cadair opened this issue Mar 18, 2020 · 32 comments
Closed

Milestones, bugfixes, oh my. #3904

Cadair opened this issue Mar 18, 2020 · 32 comments
Labels
Discussion An issue opened for, or undergoing discussion.

Comments

@Cadair
Copy link
Member

Cadair commented Mar 18, 2020

This is a infrastructure issue to solicit input on what people think the role of milestones should play.

Currently we have two bots, one which checks all PRs have milestones and one which backports PRs based on labels.

This ends up in a bit of a weird workflow (imo) where we are duplicating metadata over labels and milestones and we end up with milestones not really meaning very much:

  1. Open a PR to master, milestone it 1.0.x and label it backport 1.0 and backport 1.1 these two actions are duplicating information.
  2. Merge the PR, the bot runs and two backport PRs are opened.
  3. Milestone the backport PR to 1.1 1.1.x and the PR to 1.0 1.0.x.
  4. Merge the PRs, we now have two merged PRs milestoned 1.0.x so when we come to look at doing a release we have a bunch of PRs in there that aren't actually on that branch.

Proposal:

  1. We do not milestone PRs to master. (This needs a change to the bot to ignore certain base branches).
  2. We milestone (maybe automatically) PRs to backport branches.

This would allow us to better keep track of merged PRs for releases?

@Cadair
Copy link
Member Author

Cadair commented Mar 18, 2020

ping @sunpy/sunpy-maintainers

@Cadair Cadair added the Infrastructure Issues or PRs that affect the CI or packaging of SunPy label Mar 18, 2020
@nabobalis
Copy link
Contributor

Seems pointless to milestone things now.

@Cadair
Copy link
Member Author

Cadair commented Mar 18, 2020

The main thing I think they are worth keeping for is to have a list of the PRs that have been merged in a window before a bug fix.

You could then do three searches

  1. PRs still open for a bugfix branch
  2. PRs merged for that bugfix
  3. PRs waiting to be merged

@nabobalis
Copy link
Contributor

nabobalis commented Mar 18, 2020

Half of those would have no milestone since they pointed to master originally.

If possible, make a bot or something auto milestone backports and set the no changelog label.
But overall, it seems ok to just see open backport PRs but it seems pointless.

@nabobalis
Copy link
Contributor

Can't we get a bot to auto milestone based on a backport label or a bot to auto label based on the milestone?

@nabobalis
Copy link
Contributor

Or we go back to version specific labels?

@dstansby
Copy link
Member

dstansby commented Mar 18, 2020

My two cents:

  • The milestone should be the earliest release in which a PR appears
  • If we milestone correctly before merging, it should be possible (he says having never used bots before) to use a bot to backport to all branches including and above the milestoned branch

EDIT: Keep reading for my updated two cents...

@nabobalis
Copy link
Contributor

The labels allow us to skip versions if we need during a backport.

@dstansby
Copy link
Member

dstansby commented Mar 18, 2020

Why do versions need to be skipped? If something is milestoned to 1.0.10, shouldn't it go to the 1.0.10 branch and the 1.1.3 branch?

@nabobalis
Copy link
Contributor

In the unlikely case we have a bug in 1.0.x, master but not 1.1.x

@dstansby
Copy link
Member

Good point. Seems like we need a 1 > many mapping of PRs > versions then. Since github Milestones can only do 1 > 1, we should just use version labels to keep track of which versions a PR goes into, which we sort of do now with the backport labels.

@nabobalis
Copy link
Contributor

We used to use labels for each version and replaced them with milestones. Milestones do get us the pretty github % for the next version but its not really important.

@dstansby
Copy link
Member

dstansby commented Mar 18, 2020

Ah in the process of thinking and writing about this I think I understand @Cadair s original post. Since each PR goes into a single branch, we could milestone every new PR to the next major release (2.0 currently), then milestone the backport PRs to their respective branch release numbers. (ie. we milestone based on which branch that specific PR is going into, not the earliest version number into which the code fix is going into)

@nabobalis
Copy link
Contributor

But if we milestone all master PRs to the next major release, does that give us anything?

@dstansby
Copy link
Member

dstansby commented Mar 18, 2020

It solves

Merge the PRs, we now have two merged PRs milestoned 1.0.x so when we come to look at doing a release we have a bunch of PRs in there that aren't actually on that branch.

because in my suggested scheme we would target the original PR to 2.0.

@nabobalis
Copy link
Contributor

Ah I missed that!

@Cadair
Copy link
Member Author

Cadair commented Mar 18, 2020

I agree that milestoning the PR to master as 2.0 would fix that, but is it giving us anything over just not milestoning it?

@dstansby
Copy link
Member

Github statistics is the only thing that comes to mind...

@dpshelio
Copy link
Member

I'm not understanding the workflow... and maybe because I've always assumed that milestones are a way to put a "due date" to issues... no necessarily to pull-requests. For me marking something with a particular milestone is aim the focus to solve a problem, implement a feature, ... in a particular time frame. To prioritise work and not to work on what just pop out today (unless is something urgent that needs a new release and therefore a new milestone is created).

For PRs and the workflow we have in place, I think that the 2 points that @Cadair proposes (they aren't exclusive, right?) makes sense. As I imagine the backports need attention as they may fail, and using the milestones may be an easy way to filter and visualise them.

Still... I think we should assign milestones to the issues.. and right now all that's there are PRs.

@Cadair
Copy link
Member Author

Cadair commented Mar 19, 2020

We only assign milestones to issues that would block a release. These are normally major 🔥 bugs or infrastructure issues that actually prevent a release. The reasoning behind this is that we were always moving issues from one milestone to the next because nobody was working on them.

Given that SunPy operates on a almost entirely voluntary basis we can't really have deadlines or really even priorities on issues imo. We have been happily using milestones as a release marker for PRs for a while, but the backport bot has changed up our workflow enough that I thought it would be worth reconsidering.

@Cadair
Copy link
Member Author

Cadair commented Mar 25, 2020

From the community call on 2020-03-25 we have a workflow issue with status of bugfix releases. When considering if a bug fix release should occur we need to answer a few question:

  1. What PRs have been merged flagged for inclusion in bug fix releases (for one or more active backport branches)
  2. Do any PRs require ManualBackport before a release should occur.
  3. What is the status of CI / RTD on the backport branches and how old is that info?

@Cadair Cadair changed the title Milestones? Milestones, bugfixes, oh my. Mar 25, 2020
@nabobalis
Copy link
Contributor

I forgot what the goal of this issue is.

@nabobalis nabobalis added Effort Low Requires a small time investment Package Novice Requires little knowledge of the internal structure of SunPy Priority Low Slow action required Documentation Affects the documentation labels Jul 11, 2020
@Cadair
Copy link
Member Author

Cadair commented Aug 19, 2020

To decide how we want to use milestones going forward.

@nabobalis
Copy link
Contributor

We updated the rules for milestones and I think we are happy with them.

@kakirastern
Copy link

Good to know, such a long-drawn out discussion...

@dstansby
Copy link
Member

I often use milestones in other projects to understand what the first version of a package a given PR landed in. For example, going to #5224 there is nothing that tells me which version of sunpy this PR is in. So I would advocate milestoning stuff that is not being backported for this reason.

@ayshih
Copy link
Member

ayshih commented Oct 21, 2021

What I do for any given PR is to pull up a commit in the PR, and GitHub will show below the commit message which releases it is in. #5224 in specific is not a good example for this because it's not in any releases yet.

I'll note that a potential issue with manually adding milestones is that they may not be accurate.

@Cadair Cadair reopened this Oct 21, 2021
@Cadair Cadair added Discussion An issue opened for, or undergoing discussion. and removed Documentation Affects the documentation Package Novice Requires little knowledge of the internal structure of SunPy Priority Low Slow action required Effort Low Requires a small time investment Infrastructure Issues or PRs that affect the CI or packaging of SunPy labels Oct 21, 2021
@dstansby
Copy link
Member

👍 that works really well, thanks for the tip! I think that removes my motivation to milestone now, so we could close this again?...

@nabobalis
Copy link
Contributor

I had no idea GitHub did that, nor can I find it.

@ayshih
Copy link
Member

ayshih commented Oct 21, 2021

For example, if you pull up the first commit of PR #4659, you can see:
commit
which shows you that the PR was first released in 2.1.0rc1. You can click the "..." to show every release the PR was in.

@Cadair
Copy link
Member Author

Cadair commented Oct 21, 2021

Nice!

@nabobalis
Copy link
Contributor

That is nice but it only seems to happen on PRs merged, tagged and released from main?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion An issue opened for, or undergoing discussion.
Projects
None yet
Development

No branches or pull requests

6 participants