-
Notifications
You must be signed in to change notification settings - Fork 819
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
Project governing status #2436
Comments
Oh come on. There are only 16 open PRs right now and your oldest one is barely 2 months old. None of the maintainers is paid for the work on this style so you need to accept that available free time varies for them. Don't get me wrong - as i have said before it would be great if decisions on changes were faster, in particular when the result is ultimately a rejection. And i think this could probably be improved without additional work load for the maintainers - by doing early reviews on suitability/prospects of the changes. But the idea that this is currently a particular issue and development is stalled by maintainer inactivity is fairly frivolous. I would also like to encourage a bit more responsibility on side of the contributors here. I am currently reluctant to submit new changes because i have a fairly long list of things that could use improvement with my previous modifications and i feel it would be important to address these - even if it is difficult to find the time. I know there are some contributors here who regularly contribute followup changes and fixes for their own modifications and consider it their resposibility to do so but there are others with more or less a don't look back approach. In principle that is fine, any contribution should be welcome. But it is certainly more valuable if the contributor also looks after his/her changes afterwards. So to paraphrase a famous quote: don't ask what your maintainers can do for you, ask what you can do for your maintainers. |
PRs may have been open only for short time, but there is quite the number of open tickets that need to be addressed. I know not all of them may be done at once, but what is holding that one? #1504 I hear that this code needs to be implemented before more work comes in, yet like many other issues it is stagnant for many months. |
The way to solve issues is more contributions, not more maintainers. If interest in an issue from maintainers was required to solve it, we could close half of the outstanding issues as wontfix. We are discussing new maintainers internally, but need to ask them first. |
Still contributions seem as not the only thing required. In example issue 1504 the code is ready if I read it right. |
I'm not quite sure actually. I think there is a strong causal relation between how quickly contributions are merged, and how many contributions we get. |
No, there are open issues impacting the lua branch: lua . A specific issue is best discussed there |
I think the lua branch is kind of specific and I don't think it's relevant here. It's a big, "backbone" change and we're not having ready PRs just to merge, so it's still in a code developing stage and not a governing stage. It's where Paul is wearing contributor's hat. The only thing in common is lack of information - when I knew I could test something, I just did it, but now I'm not sure what is needed and who could help. I don't want to push anyone to do (or not to do) anything. It's absolutely not a problem of anybody being "lazy". It's not a problem of being paid - nobody is, probably, we're all volunteers here and I appreciate the work on writing issues and code the same as I do appreciate the work on deciding and merging. But when I (as a contributor) "ask what you can do for your maintainers", I simply don't know, so I feel there's kind of invisible bottleneck. My loop as a contributor is broken and I'm not even sure my work is useful here. This is the problem I try to describe here. I'm happy with the project starting to be aware of contributors community. We have CoC and we (as a project) learned how to talk to the people - that's just great! That's probably why people become active and try to do something. I see it exactly the way Matthijs said: "I think there is a strong causal relation between how quickly contributions are merged, and how many contributions we get." Communicating, deciding and merging is important for people to be properly guided and happy to do more. Managing volunteers is important and it's a part of healthy open project. |
Regarding strong causal relation between how quickly contributions are merged, and how many contributions we get: This might be the case but i don't think this project is in the long term limited by the number of contributions. If i have a look at how the map changed in styling over the last years most clear improvements are the result of slow, considerate work that was not just quickly made and quickly merged. If i have a look at the open issues a significant number of them are about problems introduced by changes that were noted soon after these were made but that were not fixed since then. This is why i called for more responsibility on side of the contributors - quality of contributions rather than quantity. If you'd just decrease the latency when merging changes and this way encourage more contributions this would (a) reduce pre-merge QA of the changes and (b) increase the number of unsolved issues with the style since it is always easier to introduce a new feature than to solve a problem introduced by a feature addition that was not recognized when the change was originally made.
Well - if you have that impression it would be ridiculous of me to deny it of course but if i look at the currently open PRs there are five from you with new feature additions (and in general of the 16 open PRs the vast majority are either feature additions or subjective styling 'improvements' in contrast to objective fixes for styling or technical issues). We have 386 open issues many of which as said are problems with previous changes with obviously also several regarding your changes. I think focusing on fixing these would be a very important contribution and in line with doing something for the maintainers. Merging a basic fix for an existing problem is much easier than weighing the advantages and disadvantages of a feature addition or a subjective styling change regarding if it is an overall improvement.
The main reward for contributing is usually having improved the map - and due to the prominence of the main OSM map and its importance for mapping this is quite a significant reward. So the question you should probably ask yourself is: have my changes improved the map and can i change the focus of my work to make it better. My assessment of your contributions would be yes in both questions - but that is my subjective impression of course. |
Yes, unfortunately I have been rather busy elsewhere. At SotM @pnorman @math1985 and I got together to discuss the situation, and I volunteered to step down entirely if the others thought it was worthwhile, but I was asked to remain as a maintainer, despite my limited availability. We also discussed appointing new maintainers, and there were three candidates nominated. I've now invited them all and have had responses from two of them so far, so please welcome @kocio-pl and @imagico to the team. Of course, this won't solve overnight all the problems, but I think it will help. I would like to re-iterate some of the points made above. I think it is worthwhile focussing on:
Personally, I will be making more of an effort to keep up-to-date with reading the comment notifications (I'm again over 250+ notifications behind) and contributing where I can to the various discussions. |
Thanks for the invitation, Andy, and congratulations, Christoph! I hope joining new active team members will help us to better manage the project in the future. I've started with a bit of cleaning and like to continue that for some time to get used to the new tools. For this moment I consider the governing status issue as resolved (very quickly!), so I'm closing it. However feel free to drop here any related comments, if you have one. |
Welcome to the team @kocio-pl and @imagico! In the past, we worked on the principle that every PR should be reviewed by one other maintainer (maintainers don't accept their own pull requests). Let's stick with this principle, at least for now. Also, in the spirit of open source development, let's keep having (policy) discussions in the open as much as possible (perhaps except for discussions concerning people). With the addition of the new maintainers, @gravitystorm is taking a step further back. I think this is a good occasion to thank him for all the work he has put in getting the migration to cartocss to work, in maintaining the project, and in expanding the community. @gravitystorm, even if you don't have time to actually contribute code, high-level guidance on cartographic direction from your side is of course still very much appreciated.
I very much agree with this. We should, as much as possible document, these changes in CARTOGRAPHY.md.
@imagico Do you have some examples of these? |
I have observed some counterexamples here and there and thought that it's not that important, but your proposal is perfectly acceptable for me.
That's exactly how I feel it and it looks like an established practice here.
Sure! I would also like to thank you for the work you've done here - not only for the code and cartographic design, but also for the team building. |
From the issues i opened recently: #1934 #2353 #2378 #2380 - of course the last ones are simply too fresh to expect a fix already (big changes with lots of side effects that require thorough consideration). But i also had in mind the footway matter (#1793, #1748) and the tracks at z13/14 - see after merge comments in #747 and later issue #1591. And the general chaos of label/icon priorities at high zooms leading to icons and labels disappearing as you zoom in, changes keep adding features there but no one addresses the basic problems. And to also appropriately kick me in my own butt - #1547, #2135 - the former hinges on water polygons of course. |
I'll just add here - please also welcome @nebulon42 to the maintainers team! |
With the increase in size of the maintainer team, we are going to try out a different review process. This only involves the reviews of pull requests made by maintainers, pull requests by others will be reviewed as always. The new process:
The new review system will be used for 3 months (until the end of June), after which we will evaluate it. |
A bit off-topic, but still about project governance: "What it feels like to be an open-source maintainer" blog post - may sound familiar for some of you. |
3 months have almost passed since @math1985 has proposed self-merging PRs, so I reopen the issue to discuss it. I'm not sure if it was really used at all and it was not too active time frame (except for lua branch merging), but at least we know this process was not abused and is not the source of new problems, which is good. I've just merged two PRs from @nebulon42 because I was convinced that was still preferred way of doing things, but it seems I haven't really understand the main principle - "A maintainer's pull request is (normally) merged by the maintainer that created it." And honestly I don't find it to be a major fault - merging by other maintainer means it's double-checked and somebody else find this change useful (not only acceptable). On the other hand it takes away a bit of maintainer's responsibility for his actions. But I think the ultimate goal is to make overall progress and current change rate is small, so I would just say that:
|
I think the main issue with merging PR's of others is that we might merge things that the original author does not feel are ready (this has happened in the past under the old system). But I don't feel strong about this aspect - the main thing for me is that the original author has the possibility to merge his PR. |
I think review requirement just doesn't work in this project. Even big changes like #2654 can have no one full formal review (only contributors are allowed to do this). Some PRs (bigger and smaller) are even not commented for weeks by anyone, which I read as just lack of interest. While it would be always good to have a peer review, this condition proved to be too strict. |
I don't believe this is correct - I'm certainly able to do reviews of PRs on other repos |
I was trying to formally invite somebody else for review and it looked like it's not possible, but I might just misunderstood how it really works. |
For an example of a review where I commented but didn't approve/request changes, openstreetmap/iD#4166 (review). I've done others where I've approved, but don't have one handy. The request a review by someone else is newer, and I haven't made much use of it. We might not be able to request a review from someone who isn't a collaborator, but that doesn't stop anyone from reviewing. |
It’s out of topic here, I know (but I don’t know where to put it else): For those who speak german: At http://www.spektrum.de/lexikon/kartographie-geomatik/ there is an online dictionary about cartography. Maybe it might help… |
Note i was not talking about the time a PR is open in total, i was talking about the time from end of development and discussion until it is merged or rejected. This seemed more recently often just a few days and hardly ever more than two weeks. If there were non-maintainer PRs during the last year ready and waiting to be be reviewed with no maintainer comments for more than a month and no stated reason why this is on hold please point me to them.
Yes, there are a lot of different matters being touched in recent discussion here and while there are some interconnections this is not all the same of course. |
What exactly is the issue with feature additions? Yeah, some of them like adding a bunch of new icons might have been over kill (I assume that's what your talking about), but adding features like waste water plants was relatively a good idea. I don't see why maintainers shouldn't review new features even if they don't like them either. Its better things are reviewed, rejected, and maybe be refined later based on the feedback then not be reviewed at all simply because its a feature and maintainers don't like those. As a side to that, one issue I find particularly frustrating is when a bunch of work is put into an issue in the ticket, but then copious amounts of further work is necessary in the PR because someone decided to jump in with an opinion there, instead of participating in the original discussion. Especially if the PR is then ultimately rejected. Maybe I'm ignorant to the difference between an issue and a PR, but to me the point where a PR comes into the equation should be when most of the design aspects of the issue have been worked out. I know that the maintainers can't follow every issue discussion, but most things are decided in issues through rigorous discussion. A lot of contributors and issue openers don't subsequently participate in the PR either. So id appreciate more clarity on what the differences between the two are and how putting time into testing things, along with listening to feedback from contributors can be properly balanced between them. In that vain, I don't see why more maintainers can't chime in at the point where the issue is created to say its something they don't think should happen. I know that can't happen in every case, but I'm pretty sure some past issues could have been rejected a lot sooner in the process then they were. There should be a stop gap to keep a maintainer from swooping in to close an issue that's about to be merged and has been open for like 4 years without anyone criticizing it. As it currently stands, @kocio-pl is the main (if not only) maintainer that both participates regularly in issues and also gives feedback on PRs. A maintainer's duties shouldn't start and end at the point of merging something or not. |
It depends on how recent moment you took. Longer delays are rare indeed, since most of the discussion is done in issue tickets and this is just the last stop, as you have noticed, but most recently even in the PR tickets we have more (sometimes unconclusive) discussions.
I hope you could tell (in the proper place, like #1975, to not go offtopic) about why do you think it's conflict at all, because I don't agree and I even don't know justification. Yet what I have seen is that mainly simple bugfixes get merged by other maintainers, but without even involving in discussion. Which is easy to understand, but double worrying. |
Maybe we have different needs and learning paths then? Here is my personal experience: I've learned from rejection of my PRs mainly that my work is not welcome and that being more stubborn and less creative is the way to go. In the end I got strong impression that mistakes are expensive and I can't afford making them in the long run. And even then it was 50/50 chance, IIRC. The best learning experience for me was - and still is - when somebody tells "this should be fixed this way or could be done in a different way - here's how". That makes me want to do more work (and more mistakes, since they are cheap). This gives me (a) and (b), but also (c) more fun, (d) better team work and (e) problem is solved. |
That’s also a question of personal attitude. I’ve contributed quite a long time to this repository before becoming maintainer, and I had PRs rejected, but I did never feel “not welcome”. I’ve tried to learn something; and, though maybe I don’t always agree, understand other people’s reasons for the reject and get a better feeling for the bigger picture. |
Trying to keep the discussion on topic - i have not made statements of policy here, i have made some explanations and observations that could help understanding why several of the maintainers are not currently doing much reviewing of changes and what might influence that in terms of work culture. Everyone is free to ignore that and everyone is free to suggest differently. For explanation why maintainers do not review changes they disapprove - this is mostly a matter of courtesy towards other maintainers who have different preferences. In a consensus based decision making system it is essential to not exercise your right to disagree when it is not fundamental for you. In other words: I expect my objections to a change - when voiced - to be taken into account in substance and not be overruled. But this in return means that when i don't want to insist on it i let others who have a different opinion make decisions i disagree with (but there none the less needs to be someone who does this and takes responsibility). Keep in mind maintainers are volunteers here just like anyone else. My motivation for doing review work (and in sum i might have done more of that before i became a maintainer than afterwards) was to learn something and get inspiration from the work of others and from discussing it with those who make it, understand their decision making process etc. In return i share my views and give people feedback about their work. That for me is the essence of cooperative design work. Others might have different motivations. My observations regarding the critical attitude concerning addition of new features of several maintainers are a statement of fact. I could support this with public statements in that direction but i don't think that's really necessary. Feel free to discuss this in a separate issue but you will need to convince people with arguments and evidence and not just a simple i like new features and i don't see why anyone would not like them. |
If the PR doesn't get reviewed, then it needs to wait until a maintainer reviews it. If there's no maintainers willing to review a given PR - even to decline it - then that suggests that there's an underlying problem that the maintainers need to resolve first. I'm not sure what you have in mind by "decisive fallback" - possibly you mean that a PR is automatically merged or declined after X time without review. But I don't think either of those would be good for the project. We have 7 maintainers at the moment, each of whom can review any PR. If we get to the stage where no PRs are being reviewed in reasonable timeframes, then we can figure out the underlying causes at that point.
Just to reiterate, all the maintainers have the ability and responsibility to review PRs, although please bear in mind as @imagico mentions, that everyone is a volunteer here, so patience and understanding are often required. |
I realise this is only a turn of phrase, but it's not a particularly friendly one. "What can I do to help with [something]?" is a better way to phrase statements, rather than "I don't see why other people don't do [something]". |
Thanks for the answer, that sets the basic framework for me. Although I still feel it's failing, it's clear for a start, so I'd like to evaluate that in the next few months.
No, I meant any fallback mechanism - what to do if that does not work? I claimed that at the beginning, that merging stuff after 1 month of lack of review is a good balance against PR clash (not too easy, to be not tempting, but still doable) and sadly that was most of the time, so the problem is real. I just didn't like to hint you anything to not start dispute if it's bad or good solution, just hear the best solution you can think of.
What is "reasonable timeframe" for you? For me it's 1 month. I guess I will ask then here what to do.
No need to reiterate something that is perfectly clear for me, I really understand that and I also feel that nobody should be forced to make a review he does not want/feels like doing - that would be plain horrible! I just wanted to remind that non-maintainers (developers, reporters) are volunteers too and their time and work is as valuable as ours, so it would be good to strike some balance we lack currently (that's my opinion). |
Maybe you guys could make @jeisenbe a maintainer to do basic tasks like label issues if he wants. It seems like those types of things are getting neglected lately and they are important to keep up in my opinion. It looks like new issues haven't been labeled in almost three months. |
@Adamant36 - thanks! But I don't think I could make that commitment at this time. I don't have a good enough server or internet connection to review PRs that require loading the planet, and I don't think I have enough experience. |
For information - @jeisenbe has now been given maintainer status on this repository. See #3846. Note - as said on various occasions before - being a maintainer is not a prerequisite for reviewing pull requests. Anyone can do and is invited to do that. Reviewing changes is not something that requires a lot of resources. Setting up a testing environment can be somewhat tricky but it does not require powerful hardware. |
True. @Adamant36 has reviewed the cartographical changes in a number of PRs and issues, and there are a number of other contributors here who make helpful suggestions, even without doing a full review of the changes. While it does take some time to download and set up the test rendering environment, it's fairly user-friendly with Docker. I'm running the test renderings via Docker on a mid-2012 MacBook Pro with 6 GB of RAM. One year ago I had never used a command-line application or Github. It took a little time to get up to speed, but I believe almost anyone can participate in improving this project, if they have the time. Not so much harder than figuring out how to use JOSM. |
I think this RFC on 'rough consensus' is a very interesting read related to our decision making process. |
Thanks, might be interesting read, but it's long. Do you think the index summarizes it well? What are your general ideas or comments about that?
|
The linked document isn't too long for a native speaker, but I understand that it might be a difficult read it all if English is not one's first language. I describes a somewhat different situation but it has a few good points, which I will try to summarize. The situation described is about the Internet Engineering Task Force (IETF), which has several Working Groups which discuss various internet infrastructure and engineering problems, and each has a chairperson. They sometimes meet in person, where semi-anonymous "votes" about issues are sometimes done by literally humming, other times there are votes taken via mailing list. This is rather different than our situation. However, some features are the same: like a working group, the maintainers here are tasked with improving this style and investigating issues that are brought up by the larger community. Sometimes decisions made here are controversial, but we strive to make decisions based on consensus, rather than by fiat or voting. Often there are trade-offs that have to be made, especially for changes to cartography. The document starts out by stating: "we don't let a single individual dictate decisions "Nor should decisions be made by a vote "Nor do we want decisions to be made in a vacuum without practical experience. "Instead, we strive to make our decisions by the consent of all participants, though allowing for some dissent (rough consensus), and to have the actual products of engineering (running code) trump theoretical designs" The last thing, about "actual products of engineering" being more important than theoretical designs is not so relevant, since we rarely have debates about what underlying code structure should be used, but about the end product: the cartography. But the other points, which describe consensus-based decision-making, are relevant
This part is key, because it defines what "consensus" means: "Engineering always involves a set of tradeoffs. "It is almost certain that any time engineering choices need to be made, there will be options that appeal to some people, but are not appealing to some others. In determining consensus, the key is to separate those choices that are simply unappealing from those that are truly problematic. "If at the end of the discussion some people have not gotten the choice that they prefer, but they have become convinced that the chosen solution is acceptable, albeit less appealing, they have still come to consensus. "Consensus doesn't require that everyone is happy and agrees that the chosen solution is the best one. Consensus is when everyone is sufficiently satisfied with the chosen solution, such that they no longer have specific objections to it." Comment: the important thing is that specific problems and objections are addressed and resolved, not that everyone thinks a certain color is the best-looking, for example. "after the initial discussion of the pros and cons of the available choices, it is most important to ask not just for objections to a particular proposal, but for the nature of those objections. ... "Following up with, "What are the reasons you object to choice A?" is also essential. Comment: This means that if a maintainer or another contributor reviews a PR or objects to a solution to an issue, they should give their specific reasons for opposing the choice, and explain why their preferred solution will solve the problem. An answer of "I don't like this" is not helpful. "a solution that has no objections, but also has no base of support and is met with complete apathy, is not a solution that has any useful sort of consensus. Consensus does require the active engagement and eventual support of those who are working on the solution." Comment: For us, this suggests that a maintainer should not merge their own PRs without a review since lack of any negative comments does not prove that the PR is a good idea. The next section is about compromise, and consensus versus voting / horse trading: "engineering always involves balancing tradeoffs, and figuring out whether one engineering decision makes more sense on balance compared to another involves making engineering "compromises" Comment: for example code complexity versus good mapper feedback versus the best rendering "However, there is another sense of "compromise" that involves compromising between people... For example, a minority ... might object to a particular proposal, and even after discussion still think it is deeply problematic, but decide that they don't have the energy to argue against it and say, "Forget it, do what you want". Comment: These are good examples. Often we will be personally happier by being friendly and offering "you can merge this PR, if I can merge mine", or thinking "it's not worth bad feelings to argue about this", but such compromises can lead to bad results for the project Summary: "Coming to consensus is when everyone (including the person making objection) comes to the conclusion that either the objections are valid, and therefore make a change to address the objection, or that the objection was not really a matter of importance, but merely a matter of taste." I’ll address the other sections later, but this is the most important one. |
TL/DR: "Coming to consensus is when everyone (including the person making an objection) comes to the conclusion that either the objections are valid, and therefore make a change to address the objection, or that the objection was not really a matter of importance, but merely a matter of taste." |
Whoops - clicked on close by mistake. Still, I'm not sure what the result of this issue should be or how we get to closing it. |
This issue has certainly drifted over time. Right now I think it's a
reasonable place to discuss how we should make decisions and what it
means to be governed by consensus.
|
For me it doesn't matter if it will be this ticket or a new one. On the one hand the title is perfect, on the other hand this might be too much to scroll (already 108 messages) and over 3 years the actual problems has changed few times, so a follow-up ticket might be OK. |
Closing, since the original concerns about lack of sufficient number of maintainers has been addressed for now, and the discussion about consensus-based decision making can continue in #3951 |
I'm a bit worried about governing status of this project.
This is not only current problem, however long and stalled PR queue is the most visible effect. This alone would not bother me if we have ongoing discussion - it's perfectly clear that some code needs more insight and debate, and I consider the time to do this as the most important part of review process. But in many cases we have passed the point of oblivion - discussion is no more active and it's getting hard to recollect the arguments. Moreover decisions are not being made and the code is not merged, nor even dismissed.
We kind of stopped moving, which is bad for any project if it's not close to be perfect. And we're not there yet - for me there are a lot things to change and more people joining us also tells me we have nice potential here. With osm-carto community getting stronger the problem of project governance model is getting more and more evident for me.
We have 4 collaborators made equal by the (repo) creator. Yet @gravitystorm is in practice not active here for a long time, rarely even giving some advice or expressing his thoughts on general issues. @matkoniecz is not committing anything for about half of year now, which is also quite long. @pnorman is recently more active, but he tends to follow purely technical issues (like code quality, performance or general database framework). Which leaves us with just @math1985 as the only active collaborator trying to cover all the fields, like communication, decision making and merging code. With his activity drop for 6 weeks it's no wonder this project is stalled.
I'd like to be perfectly clear - there's nothing wrong with people activity shift (or even going away) in open projects. The real problem is that our governing structure does not reflect the reality. Even if Matthijs will be back with the same superpowers, it's still one man the project relies on for day-to-day operations. In my opinion this is not healthy situation.
What are your thoughts on this topic?
The text was updated successfully, but these errors were encountered: