Suggestion to slightly improve Julia development

Community
81

I think it would be great to include bump etiquette specific to the project in CONTRIBUTING.md.

12 Likes
82

One observation I have here is that it’s quite easy for PRs to “get lost in the shuffle”. If someone doesn’t happen to review it and give necessary feedback or if the author forgets about it for a while, it’s very easy for it to fall by the wayside. A way to address this would be to have a state machine for PRs that is partially automated and partially manual with clear roles. Here’s my attempt to write down such a state machine. This only covers PRs from non-committers since committers can shepherd their own PRs and my impression is that we do not have a problem with committer PRs languishing.

initial state

  • automated:
    • top GitHub-suggested reviewer is automatically assigned
    • any maintainer can reassign at any point in this process
  • transitions:
    • pr review label automatically added

pr review

  • manual:
    • assignee will be pinged until they review
    • assignee’s review is definitive
  • transitions:
    • PR merged by any maintainer (done)
    • PR closed by any maintainer => rejected
    • PR review request changes => changes needed
    • PR review accept => accepted
    • PR closed by author => abandoned

changes needed

  • manual:
    • author will be pinged until they close or request review
  • transitions:
    • new changes pushed, “please review” commented => pr review
    • PR merged by any maintainer (done)
    • PR closed by any maintainer => rejected
    • PR review request changes => changes needed
    • PR review accept => accepted
    • PR closed by author => abandoned

accepted

  • automated
    • waits until all CI actions are finished
  • transitions:
    • CI passed, no conflicts => auto merge (done)
    • CI passed, conflicts => changes needed
    • CI failed => ci review
    • PR closed by author => abandoned

ci review

  • manual:
    • assignee will be pinged until they review
    • assignee’s review is definitive
  • transitions:
    • PR merged by any maintainer (done)
    • PR closed by any maintainer => rejected
    • PR review request changes => changed needed
    • CI restarted => accepted
    • PR closed by author => abandoned
29 Likes
83

This is awesome! A set of well defined rules for each state will make the workflow more understandable for those who are starting to contribute.

Is it possible for the same bot to ping people every, let’s say, 15 days if no activity is seen in the PR? Or is it too spamming?

84

If you read the descriptions of the manual states, pinging people periodically when manual action is required is part of what is described.

85

I think I did not understand the text. I interpreted the manual as something that should be done manually by someone, and not something automatically done when a manual action is required. Hence, I thought your description meant that we should be pinging those people manually.

1 Like
86

Those are states that require manual action by someone. When the PR is in that state, the bot will ping the person who needs to do something.

3 Likes
87

Now it makes much more sense :slight_smile:

1 Like
88

Could this cause a positive feedback loop that puts a very heavy load on a small number of reviews? I don’t know how github suggestion mechanism works, is it based on who last touched the updated code, or simply who frequently reviews PRs?

4 Likes
89

Please consider closing very old PRs (eg >1 year) automatically. They can always be reopened if there is continuing interest, but about 20% of the nearly 1k open PRs seem to be languishing with no activity.

4 Likes
90

The idea is that the originally assigned persons would just be responsible for picking someone to actually be assigned the PR. You could almost pick a maintainer at random for initial assignment. Anyone who isn’t active enough to delegate a PR to a good reviewer should be moved to the dormant maintainers list. Another possibility is to use tbe code owners mechanism.

1 Like
91

I think the automatic pinging would have the effect that this wouldn’t happen anymore. There are quite few people who watch everything on the repository (myself included), so if somone hasn’t responded in a while, we can just close it. The alternative is to have a bot warn that it’s going to auto-close something and then you have to keep telling it not to. I’ve played that game with several bots on upstream repos and it’s annoying. Instead of just getting a reminder, you have to reply that you’re still planning on finishing.

2 Likes
92

If you are pinging on Github, my understanding is that a user can also just turn of all notifications from a repo if they don’t want to get them any longer; effectively entering into “I really mean to finish that PR, once I get around to it, possibly in 2025 (or 2525), just don’t want to get pings” mode.

But maybe you are right, let’s see.

1 Like
93

I have been thinking about the state machine approach — I have looked at old issues/PRs, and a lot of them seem to be dormant because they are pending on some architectural decision that few people are qualified to make, or need a major API reorganization for consistency.

Perhaps a state could be introduced for this, with a lower ping frequency (eg every 2 months).

4 Likes
94

Yes, that’s a good idea.

1 Like
95

This thread was suggested to me while creating a different one, and although I just skimmed through it, I thought I’d share my experience that was wonderful with respect to the speed of reply, review and communication. Got one suggestion within one day, implemented it, got approved 5 minutes afterwards.

The speed of build and test and CI parts was horrid, but that’s the different thread :slight_smile:

3 Likes