astropy / astropy-project

Documents and policies regarding the Astropy Project as a whole.
Creative Commons Attribution 4.0 International
36 stars 43 forks source link

[Proposal for Discussion] New Roles for General Project Maintainers and DevOps #136

Closed kelle closed 2 years ago

kelle commented 4 years ago

Following on discussions had in person and most recently, in #118 , I would like to use this issue to discuss @bsipocz 's proposal of adding a role for Project Maintainers. These are folks who have their hands in many parts of the project and understand how lots of things work. Several pros and cons to this idea have been mentioned and I'd like to encourage folks to summarize their thoughts here again even if they were recently expressed elsewhere (@bsipocz, @eteq). I also think it's ok to go ahead and name the folks who you think fit the bill of this potential newly named role. (I can think of @pllim and @bsipocz, for example.) To try and foster the most discussion possible, I'll also post a link to this issue in the other relevant forums including the slack #project channel and the astropy-maintainers email list.

pllim commented 4 years ago

I like to think of my work to be something along the lines of operations, quality assurance, reliability, and some customer support. Perhaps a subset of devops though I have to admit I never grokked what "devops" really means. 😅

bsipocz commented 4 years ago

Yes, what the two of us do is more along the lines of operations and devops, a role that requires a good enough global oversight for the project and the wider ecosystem, the roadmap and downstream usage. A welcome change would be that it's an acknowledged role so the subpackage maintainers don't take it as a stepping on their toes into their territories when a PR is being overtaken, wrapped up or reviewed. Most of the time the intention is the speedy turnaround on PRs that don't require the deep dive of the subpackage maintainers to they can focus on the larger ones.

While the general maintainer category in my head has names of e.g. Stuart M, Derek, Anne. Who are willing to go into a problem in several submodules but generalist for to overtake the maintenance of submodules. This role can also be a stepping stone while a new maintainer gathers more confidence in the workflow and review process. A natural benefit would be that submodules can be thus tagged under this general, e.g. samp, wcs, etc, which actually better describe their status, or desired status (for the case of wcs).

On the other hand I don't propose to overpopulate these roles. I firmly believe that people should be removed from roles when they don't perform said roles for years rather than kept being listed (if consistently there is no effect in pinging them to do a review or release or provide input, then what's the point of the list?). The current page certainly has that problem. Other projects use the "emeritus core developer/emeritus maintainer" category (https://scikit-learn.org/stable/about.html, https://numpy.org/gallery/team.html) which better describes the situation.

kelle commented 4 years ago

Excellent! I hear a 2nd concrete proposal: a new "devops" role. I will update the issue accordingly.

(I hear you about taking people out of named roles but I'd prefer we keep this issue limited to discussions about the new roles. You are welcome to start a new issue to discuss the retirement plan.)

hamogu commented 4 years ago

Sorry, I accidentally edited the title here to remove the "Devops" that @kelle just added (and replaced it again 2 min later). I realized that me editing the title is inappropriate for this issue (neither did I open it, nor am I an a position to decide this).

That said, I think these are two different things and should be different issues. I suggest to revert this issue to "general maintainers" and open a new issue for DevOps, if we don't have one. Those are two totally different things and should not be lumped together.

As for @bsipocz original proposal: I think that's great idea. I suggest that those general maintainers have the same access and commit rights as sub-package maintainers, i.e. they can add/remove labels, approve PRs and merge PRs following this line in our (draft) policies: https://github.com/astropy/astropy-project/pull/100/files#diff-5d169bd40a8f4276b638234544a4521fR21

Essentially, "general" maintainers" can do everything that subpackage maintainers can do outside of their own sub-package. That automatically means that every sub-package maintainer is also a general maintainer, so in my interpretation of what @bsipocz suggests, @dhomeier would not be a "general maintainer" because he already maintains a sub-package.

Now, this is where the DevOps mixes in - I suggest that as a policy, all "DevOps" also have (at least) "general maintainer" rights (I think right now, all DevOps have the required commit rights anyway?).

hamogu commented 4 years ago

Found the issue: Role for DevOps belongs more into #118 than here. Or could be it's own issue, since #118 is more general than just "create a formal role for".

bsipocz commented 4 years ago

or even better Derek and Stuart could choose whether they are happy with the subpackages they are currently listed, somehow shoehorned into, or rather be listed as general ones. I feel they both do more than their modules, but it should be up to them.

devops certainly need commit rights, but I would argue admin (maybe "maintainer") rights. There have been plenty of cases when access to tokens, webhook, etc were required.

adrn commented 4 years ago

:+1: to the idea of adding a "General maintainer" role, but maybe with a name to emphasize that it pertains to the core package, like "core package maintainer"?

bsipocz commented 4 years ago

I'm not picky on the name, and even the "core package maintainer" better describes it. "general project maintainer" might be a more holistic role that we don't have and should not have before figuring out the coordination of integrations for coordinated packages.

hamogu commented 4 years ago

It could be defined in terms of what it's not. On the website, change "subpackage maintainer" to "core package maintainer". Above or below the individual subpackages add "no specific sub-package" and list the names behind that.

mhvk commented 4 years ago

Very much in favour. If asked without seeing the discussion, I'd actually have thought "general maintainer" would fit @pllim very well - general sense of most modules, how everything works, etc. Though the "keeps everything rolling" aspect maybe falls more under 'devops'. Hmm, I also don't really know what it is ... looking at wikipedia ... not really any the wiser ... Anyway, happy to go with it if @pllim thinks that is most appropriate!

Indeed, relatedly, one possibility to get ahead of this in future would be if the e-mail survey we do of people to see whether they themselves feel they are still involved had an option "other" under the question that (I think we have) about which role they fulfill, so they could self-define. (So, immediate consequence: since @pllim considers herself "devops", let it be so!)

Cadair commented 4 years ago

On this vs "devops":

I think that there should be a group of people entrusted with the infrastructure maintenance, or at least a well defined subset of it, and that should be separate from a "core package maintainer".


On generalist vs specialist:

I see no reason why one can't be both. You can be happy to dive into generic tasks over the whole code base for maintenance etc and also take on a specific responsibility for the nitty gritty and features of a subpackage. For instance, I now (somehow) understand WCSAxes quite well so am happy to be a visualisation maintainer, but I wouldn't know where to start adding a feature to samp or something. However, I would happily dive into samp and update a dependency or something if it were causing trouble. That to me is the difference between the general and the specific here, it's the in-depth expertise in that subpackage.


On reviews and merging:

One thing that I am not clear on personally, and I don't understand how this proposal would interact with, is what the expectation is for people to use the merge button on subpackages they aren't maintainers on. As I am listed as a visualization maintainer should I approve / merge a simple PR to the io subpackage? Should merge an approved PR to coordinates?

If I were also a "core package maintainer" would the answer to those questions change?

kelle commented 4 years ago

Great convo!

@Cadair , Do I understand you correctly that you are proposing a third new role dedicated to "infrastructure"?

I actually thought we had this role, but looking at the team page, I don't see it! It looks like we tried to break out the various infrastructure pieces into their own roles.

If I do understand you correctly, could you expand on what that role entails and the folks who think are currently filling that role? (I'm fine with using this thread since all of these roles are so related, but you can also start a new issue if you prefer.)

Cadair commented 4 years ago

I think others were saying the same thing here that we need devops/infrastructure people and that should be a different role to the "core package maintainer" we are also discussing here. (So I think we are still only two roles). I have more thoughts on infrastructure stuff but I think they need more time in my head.

dhomeier commented 4 years ago

I haven't really thought that much about my formal role, but probably find my position best reflected by @hamogu's idea that a sub-package maintainer's role encompasses that of any “general” or “core” maintainer, and @Cadair's note on “ generalist vs specialist”. A bit more specific for my situation, I currently foresee perhaps more work I can contribute to in io.fits, io.misc, table, modeling or even timeseries or wcs than in io.ascii, but could easily continue to work on that in my formal role as io.ascii maintainer. Also, looking at the definition of maintainers' roles in terms of PR review, taking responsibility for hitting the “merge” button etc., I fell perhaps qualified to assume an additional sub-package maintainer role in one of the first 3 sub packages above, but certainly none of the others at this time.

pllim commented 4 years ago

I forgot where I saw it in the discussions now (which seem to have spread over several different issues), but someone (maybe @embray ?) suggested a table mapping names to expertise instead of the other way around. We actually do that internally for my team's help desk and I find that useful.

embray commented 3 years ago

I forgot where I saw it in the discussions now (which seem to have spread over several different issues), but someone (maybe @embray ?) suggested a table mapping names to expertise instead of the other way around. We actually do that internally for my team's help desk and I find that useful.

That was here: https://github.com/astropy/astropy.github.com/issues/323#issuecomment-689802741

I'm sort of trying to kill two birds with one stone here:

I'd be willing to work on the code end of this, once we have agreement on a path forward. I think that my discussion in this comment is somewhat orthogonal to the above issue though.

astrofrog commented 3 years ago

I really like this idea and we could even for each person keep a record of former roles?

astrofrog commented 3 years ago

Having read this and the discussion in #118 some more, I can see the arguments for a general maintainer role that @bsipocz is making. I quite like @embray's way about thinking of it in terms of the fact that we have people and some of them have specializations.

So maybe for the core package, we could list all the maintainers, and then list their 'specialization' if any (but not all would have specializations). Essentially all maintainers would be general maintainers (which is basically how the GitHub permissions work anyway) but specific sub-packages would still require approval from a 'specialist'. Speaking for myself, I'm quite happy to dive into any part of astropy but only comfortable approving for certain sub-packages.

So then we wouldn't need to create a new separate role but instead list all maintainers for the core package and keep track and display for each one their areas of specialty. We could then still provide a way of dynamically making a table mapping sub-package to people for convenience of figuring out who to tag for review but the default view of the table would be the one listing people with optional specializations.

Then separately from that, we should make a devops/infrastructure team/role that basically has admin access to all repos in the organization.

astrofrog commented 3 years ago

I've opened https://github.com/astropy/astropy.github.com/pull/400 for the DevOps role. The other part of reorganizing sub-package maintainer roles is a bit more work and I think @embray was volunteering to take a look at that.

embray commented 3 years ago

On Tue, Sep 29, 2020, 13:44 Thomas Robitaille notifications@github.com wrote:

Having read this and the discussion in #118 https://github.com/astropy/astropy-project/issues/118 some more, I can see the arguments for a general maintainer role that @bsipocz https://github.com/bsipocz is making. I quite like @embray https://github.com/embray's way about thinking of it in terms of the fact that we have people and some of them have specializations.

So maybe for the core package, we could list all the maintainers, and then list their 'specialization' if any (but not all would have specializations). Essentially all maintainers would be general maintainers (which is basically how the GitHub permissions work anyway) but specific sub-packages would still require approval from a 'specialist'. Speaking for myself, I'm quite happy to dive into any part of astropy but only comfortable approving for certain sub-packages.

So then we wouldn't need to create a new separate role but instead list all maintainers for the core package and keep track and display for each one their areas of specialty. We could then still provide a way of dynamically making a table mapping sub-package to people for convenience of figuring out who to tag for review but the default view of the table would be the one listing people with optional specializations.

This ties into another plan I've had in mind, but haven't discussed yet, which is to have the astropy-bot automatically assign reviewers (to PRs) or assignees (to issues) based on labels and/or keywords/subpackage names mentioned in the issue. I had in mind that this would only be if the people in question agreed to such automatic assignment. And it wouldn't necessarily require that they be the assignee, as they would be free to later remove themselves or assign someone they feel is more appropriate. But the idea would be to at least give each issue/PR an initial point of contact responsible for responding to or triaging it.

pllim commented 3 years ago

astropy-bot automatically assign reviewers (to PRs) or assignees (to issues)

This idea had been floated around before, but we got stuck with the question of what if the reviewer/assignee simply ignores the assignment? When they are not on payroll, the Project cannot really make them do anything; they have to be voluntary and proactive. Even with occasional manual assignments, we still have those issues/PRs sitting there for years (people forget or they don't have time).

mhvk commented 3 years ago

While I know I would be more inclined to have a look if pinged by @pllim than I would be for a bot, I can see the advantage of sharing the work of having at least an initial look.

But it might be good to have a broader look at stalled issues/PRs for why they are stalled. For PRs for me it often is a case of a not very interesting problem combined with a poor implementation where it would take more work to get it right than to do it myself, and where I'm not sure the author is actually really interested in being an astropy contributor or just wants to get some github/GSoC cred.

In that respect, a totally different idea might be for the template to suggest that, for people who are raising an issue for the first time or are making a first PR, to briefly introduce themselves and how they use astropy. My mood definitely changes if it is a student just learning astronomy, or someone tackling some particular problem.

embray commented 3 years ago

what if the reviewer/assignee simply ignores the assignment

It could happen, sure, but that's no different than if there is no assignee and the issue also gets ignored. This doesn't necessarily free anyone else from having to help triage; it just gives an initial point of contact for the issue. If someone else wants to take it over nothing is stopping them.

The bot would also continue (as it currently does) to flag issues that aren't getting attention. I also plan to add myself (or whever is currently in the "Software Operations Support" role) to be automatically mentioned in issues that aren't getting attention, as well as issues from first time contributors.

embray commented 3 years ago

@mhvk

In that respect, a totally different idea might be for the template to suggest that, for people who are raising an issue for the first time or are making a first PR, to briefly introduce themselves and how they use astropy. My mood definitely changes if it is a student just learning astronomy, or someone tackling some particular problem.

Hold that thought. Because as I've also mentioned I want to have the astropy-bot make an automatic greeting message to first time contributors, and while I have some ideas for text of that message I definitely plan to request feedback and suggestions for the wording. Something like this would be an excellent addition.

mhvk commented 3 years ago

I think a risk with an issue being assigned is that it would actually stop other people from looking at it. But with a bot checking that any reply was made, that might well be circumvented.

Overall, I see little problem in trying it. For me at least, it has been somewhat unexpectedly helpful to have a bot warning about and then closing out-of-date PRs.

pllim commented 3 years ago

While I know I would be more inclined to have a look if pinged by pllim than I would be for a bot

There is nothing stopping me from letting the bot post as me. 😏

eteq commented 3 years ago

re: @astrofrog

So maybe for the core package, we could list all the maintainers, and then list their 'specialization' if any (but not all would have specializations). Essentially all maintainers would be general maintainers (which is basically how the GitHub permissions work anyway) but specific sub-packages would still require approval from a 'specialist'.

I like this idea phrased this way! You're right it's more reflective of reality. I think it wouldn't require a ton of change anyway since all the sub-package roles still have to be maintained as "specialist", but it allows space for "generalists".

One clarification, though: would all "specialists" also be "generalists"? I know some people have expressed hesitation to be a "generalist" but are more willing to enter via the "specialist" route. So that spoils the simplicty of the above a bit (even if we can't have it really reflect reality given the github limitations).

astrofrog commented 3 years ago

I think all specialists would also be generalists, although whether and when they choose to exercise that right is a different matter. It's going to be hard to define generalist anyway in a way that means X is a generalist and Y is not. I'll happily try and track down a bug in coordinates or WCS but I might very well fail and I don't think I could delve into votable in the first place - does this mean I'm not a generalist?

At the end of the day all specialists actually have generalist permissions in the core repo anyway?

bsipocz commented 3 years ago

I would push back on listing everyone as a "generalist". The whole point in this (and in my view of the roles page) is to know who are in charge, who are responsible for certain parts. If there is no use of pinging those people then no point in listing them. E.g. if one is only comfortable with doing maintenance of a limited scope in core then they should not be listed as a generalist.

eteq commented 3 years ago

More broadly, though, on

a table mapping names to expertise instead of the other way around.

I think the current table has been important for at least some of the recruits we have brought in over the years (although certainly not a silver bullet, but every little bit helps...), particularly when behind-the-scenes politicking is required.

But given the overlap of roles and people, it does indeed make a lot of sense to have this mapping go both ways, and it's probably easier to maintain the way @embray said! Should we make an issue over at astropy.github.io for this?

bsipocz commented 3 years ago

e.g. in my view the generalists has an overarching view of things. E.g can be pinged with any C code related issues, can be pinged with install issues, can be pinged with not so clear downstream issues. And most importantly they are comfortable with the idea of triaging issues for multiple modules not just a niche one that's their speciality. It comes back to my usual mantra, that there is little point in diluting roles with people who are not doing said role, never respond to pings related that role, as it only creates overhead when trying to find solutions, etc.

bsipocz commented 3 years ago

I think the current table has been important for at least some of the recruits we have brought in over the years

the current table is also responsible of filling roles that didn't work out, a role got filled with politicking, but not so much action followed.

mhvk commented 3 years ago

:+1: on "generalists" as a true separate category - it doesn't mean the "specialists" cannot jump in sometimes, but I think it helps give appropriate credit.

astrofrog commented 3 years ago

Ok I'm convinced and I drop my suggestion to consider all core devs to be generalists 😀

bsipocz commented 3 years ago

it doesn't mean the "specialists" cannot jump in sometimes

oh, certainly. And this is basically the main reason I don't really like the idea of assigning issues to people as it lowers the likelihood of someone taking up that issue randomly (while also puts pressure on the assignee, and that's not OK if they are in the project on a voluntary base).

But something has to be done though as there isn't any momentum by the maintainers to do regular triage. Maybe it's regular work together times, maybe it's regular mentions by the bot, maybe something else? What currently is visible that there isn't a visible drop in the number of bug fixes or closed issues even though there are 8+ contracts to maintainers for which the mantra is that the contracts are to do "tasks that volunteer effort has been unable to address, due to a lack of time or interest". It's not something that a single, magical person will ever be able to resolve when most issues need something to be said by the subpackage maintainer.

mhvk commented 3 years ago

I loved the idea of the regular work-together that you started, but reality clearly intruded as I think I took part only once. One focused on triage of specific modules sounds good, though...

kelle commented 3 years ago

Just a quick clarification, and apologies for the off topic post, but most of the current contracts are for work which volunteers were already regularly doing and not for

"tasks that volunteer effort has been unable to address, due to a lack of time or interest".

We're definitely moving in the direction in having more contracts address the neglected tasks, but those are not the majority of the contracts at the moment.

If anyone wants to discuss this further, I suggest we use #113.

bsipocz commented 3 years ago

I've copy pasted that from the Moore report. If the maintainer contacts are not for doing maintainers' tasks (triage was always part of it), then at least it should not be reported back as a thing that's got done with the grant.

hamogu commented 2 years ago

At the developer telecon, we discussed this and there is broad agreement (see dev telecon notes from today for detailed discussion) to move ahead.

Plan:

  1. create role on website
  2. add people to those roles

APE0 puts "creating roles" into the realm of the Coco, so I'll bring it there, but leave this PR open until it's done.

hamogu commented 2 years ago

Closing this since PR to add general maintainer role to website is now open: https://github.com/astropy/astropy.github.com/pull/475