Good answers. I think creating such document starting with what was already said in email would be beneficjal for new contributors. Or maybe adopting something from other OS project?
In terms of limited reviewer's time. Maybe a tiered review process would be possible, where "lower rank reviewers" would approve simplier PRs. sob., 28 sty 2023, 19:12 użytkownik Michael Bien <mbie...@gmail.com> napisał: > Hi Lukasz, > > inconsistencies exist because a lot of the things you listed depend on > the concrete case. If they wouldn't exist we could write a CI script > which does PR reviews or ask ChatGPT to approve them. > > That is the reason why it would be good to post on the dev list first if > you aren't sure. I tried to give an example in the thread I just opened. > Some cleanups have to be split, others we might be able to slip into > NetBeans in one go - there is no right answer. Timing is also important > since we usually want to avoid merging large PRs while a NB release is > baking. > > We obviously do want new contributors. Yet we also want to prevent > disappointment of eager && motivated contributors opening 10 PRs in > advance and then having to close them one by one again due to various > reasons. > > here an example: > "i want to run refactoring X, it turns out it touches 500 files, does it > make sense? how should I split it?" > > Possible suggestions could be: > - one PR with multiple commits > - several PRs > - only running the refactoring selectively > - or even that we rather not want see that particular change > > maintainers might also have different opinions on this since apache > doesn't work like the borg collective. Maybe someone is willing to sit > down with a coffee and review a big changeset, others might not willing > to do that - both can be valid and depends on the concrete changeset. > > > but let me answer a few more of your points inline > > > On 28.01.23 16:19, Łukasz Bownik wrote: > > Hi. > > I read the latest discussions and as a relatively new contributor I got > > contradictory guidance/experience. What I generally refer to is (in no > > particular order): > > > > 1. "we welcome contributions" vs "we want less noise" > > 2. "we want small PRs" vs "we want big PRs" > > 3. "we want small PRs" vs "we want to save CI resources" > > this is already covered. Both sides are correct - there needs to be a > balance in the force (and source). > > I am also trying to find out if it is possible to bind the workflow > approval logic github has to commit rights. The default only asks for > approval for first-time contributors - which isn't optimal. Apache > projects are configured differently so this may or may not be possible. > > A PR sync runs tests for 4-10h. This is a lot of CPU time. > > > > 4. "we want new tests" VS "we already have too many tests" > > 5. "our tests are not good enough" vs "out tests already run for hours" > > we have still a lot of tests which are not hooked into CI. Whenever I > see a case like this I try to reactivate those if possible. > > Realistically we also wouldn't be able to test everything due to > resource constrains. So yet again we have to compromise and be smart > about it. Some module might indeed have not enough tests - others might > be fine. > > Test coverage is also only an indicator where a project may or may not > need more tests. It is not something we try to have at 100% since its > not realistically possible and it would quickly lead to tests testing > trivial stuff for better numbers. > > > > 6. "we want to improve our tests" vs "test improvement PRs are noise" > > 7. "we want to keep code clean" vs "code cleanup PRs are noise" > > 8. "we welcome new contributors" vs "we want high value/impact PRs from > day > > one" > > both sides are true and/or wanted. Depends on the context. > > I personally would not advice to rewrite tests unless you wrote them > yourself or are very familiar with the module. The benefit of a rewrite > is also very likely minimal when compared with the review requirements > and risks. > > > > 9. "we want contributors to talk to to us" vs "we ignore emails from > them" > > This can happen. I do my best to answer mails whenever I can. I do this > in my free time like many other devs here which is limited. > > > > 10. "we want 100% backward compatibility" vs "we let plugins to disappear > > from the 'available' list all the time" > > Changing public APIs has to be avoided if possible. NetBeans is an IDE > with third party plugins, an application platform and recently also > became a language server for other editors - so a small change has the > chance of causing a lot of trouble. > > Plugins need to request verification to be available from the main > update center of a new NetBeans release. If you see something > disappearing its likely because of that. Maybe a maintainer didn't > request verification for the plugin. The reminder mail just went out a > week ago so if you have a plugin, please check if it works with NB 17rc > and press the "request verification" button. NetBeans does now also have > more verifiers. > > > > > > So I think it might be a good idea for the core theme to sit, discuss > > we are doing this right now, I just started a thread a few days ago: > > https://lists.apache.org/thread/f7mt7k3rb11sgjj1fo2vlflczvw3oycx > > > > and > > create a document ironing out these issues and laying a general process > > that should be followed by contributors. > > maybe. Most contributors who want to fix a bug or tweak something in the > IDE won't really be affected by this whole discussion since it doesn't > apply to them. So the big question is what to put into a guide and what > to omit without making it look too scary. > > best regards, > > michael > > > > > > Best regards. > > >
