Suggestion to slightly improve Julia development

I mean, the “community review requested” should be added by who has permission and decided that this is a “trivial” PR that can be reviewed by community. Something like the “good first issue”. Hence, people in the community knows what PRs are good for being revised by someone with a little more experience.

Of course, in this model, we still needs a core dev to approve the changes. Let’s hope that this model reduce the “time without feedback” we see from time to time :slight_smile:

4 Likes

@fredrikekre mentioned we already have a welcome to new contributors in the the issue template but I do not think that happens in PR’s. We can do something like: https://github.com/JuliaLang/www.julialang.org/issues/1096 and then mention to people that if they want a review from the community, to cross post their PR to the “community-reviewers” channel on Slack.

Constraints:

  • Compute time (not sure how scalable this is approach is. The core Julia repo uses zero GitHub Actions right now, likely by design)
  • Only works on the first PR/contribution someone makes.
1 Like

This is very nice! Thanks!

1 Like

I would add that there are actually quite a lot of people (some 60+) who have the ability to merge PRs on Julia, but few who have the time to do the review. I’m not sure what to do about that—time is limited and Julia isn’t backed by some megacorp that can pay people to work on these things. But if people who don’t have commit bit want to help out with review, that would be helpful.

For myself I looked at the verbose testset PR multiple times over the year. The concern I always had about it was “Why do we need this feature?” and “Is there a better way to do this?” But those questions take a lot of effort and thought to answer. In the end the feature seems harmless and potentially useful and it was easier to merge than have a debate about if we really needed it. Frankly, I’m still not convinced that we do but I think posting that might have been even more discouraging. I still think it would probably have been better to do this with some sort of verbosity setting, but understanding the problem enough to formulate that sort of position is already quite a bit of effort.

9 Likes

Hi @StefanKarpinski, thanks for the feedback.

I fully understand. If I am right, then this can be improved with the help of the community. We can try that suggestion of @logankilpatrick, but a further improvement would be to create a group of “Community reviewers”. Like, people that have made any contribution to the language, for example.

Don’t ask me why, but open-source projects usually do this. It may seem silly at first glance, but when you create a group that people apply to and get selected, you can provide some recognition. Similar things are used all over the internet. It makes newcomers more likely to keep involved in the project, which seems important since, as you said, you do not have ( yet ) some megacorp backing Julia development.

Please, don’t get me wrong. This has nothing to do with that particular PR. I used just to show an example. I have been using Julia since 2013. I know how you devs are busy because I know what is on the timeline and how difficult it is to implement. The problem is that a new developer might not understand that and just think, “they don’t want me here”.

Side note: that feature was important to me to see if I really included all my package tests that have like 4 or 5 nested levels :smiley:

2 Likes

I went through the first 15 pages of PRs and about 10% of them don’t have any reply. A few are simple doc or convenience changes (e.g. this one from a year ago).

On later pages you can find some stuff that are outdated and could be closed (e.g. this bug has been fixed elsewhere it seems).

I think if people go over the open PRs and leave comments, it could help to resolve some of them and clean things up.

3 Likes

There was a certain holiday last week in the US.

7 Likes

It would be great if more people can review such PRs. I periodically go through open doc PRs and get them merged, but this one was not labelled. I don’t know if github has a role for more people to be given permissions to label issues and PRs (but not full commit access). That could greatly help.

A great help would be more folks reviewing open PRs, and bumping them if they are ready to go. Especially the simple ones. An occasional bump will help drawing attention. For example, it would have been great to add these comments you made here to the relevant issues and PRs.

-viral

14 Likes

Yes, GitHub has a moderator role. It would be good to be quite liberal with giving that out.

11 Likes

Since this is unwarranted, it may just be a problem that better communication can resolve.

2 Likes

JupyterHub has automated welcome messages, I got one recently:
https://github.com/jupyterhub/the-littlest-jupyterhub/pull/636

Would something like that help to make new contributiors feel more welcome? Of course, it needs to be followed up eventually. Maybe the welcome-message could state something along the lines that it might take longer for a review and that it is ok to bump a PR after xx days.

9 Likes

Yes! This is a very interesting approach. In fact, I think @logankilpatrick was testing something in those lines.

2 Likes

I’d recommend looking into how Rust does a lot of this stuff as they have some useful tools they’ve used for years: Rust infrastructure can be your infrastructure | Huon on the internet

7 Likes

Hi @johnmyleswhite,

Thanks for the article! Rust is definitely an example of a successful community. I know that implementing a bot to handle those things should not be trivial, but I would like to make a proposal. Maybe in the future, we can have this implemented:

  1. A bot, like Homu will handle the PRs.
  2. When a PR is made, the bot ping the core developers / maintainers.
  3. If the a core developer thinks this is not something that requires review from an experienced developer (no API or breaking change), they add a “community-review” label.
  4. In this case, the community looks at the PR, and the bot waits for 3 (or whatever other number) of acceptances from the community. Here, the community can be anyone who already got a PR approved in the repository.
  5. After 3 acceptances, the bot merge the PR only if all the test passes.

It is a way to reduce a little the burden of polishing some areas from the core developers :slight_smile:

8 Likes

I think that most core devs are already “watching” the repo on Github, so they get notified about new PRs.

I think that this would lead to chaos. A lot of PRs and suggestions/feature requests that were rejected had more than 3 supporters (judging from the :+1:).

Designing a computer language requires a small number of core architects with a mostly consistent vision about how it should evolve — this is not something that you can simply leave to CI. This is the real bottleneck, and it is difficult to work around since it is a resource constraint.

I think it is better to just recognize the fact that API changes to Julia and the standard libraries require careful consideration with a corresponding lag. This is not necessarily bad — decisions about APIs sometimes need time to mature.

The solution is to put everything possible in packages. There is nothing special or magical about Base or the standard libraries — for example, while it is great that Test is available, development of testing frameworks can happen much more rapidly in third-party packages.

10 Likes

This was not the suggestions. No one suggested to make API changes merge by popular vote and passing CI. It was explicitely stated that it is about minor things:

Not true, even if repeated hundreds of times. Something in Base is known to not break in the next julia
version. There is a guarantee that this will not break. This is not true for packages and actually
happens in the wild for popular and important packages (hello Cxx.jl).

4 Likes

Exactly!

Yes, I also agree with this. Even if you put everything you can in packages, there will be bugs, minor improvements, documentation fixes / additions, etc. in base. My proposal would help core developers in those minor things.

1 Like

To add some anecdotal evidence, neither of my documentation PRs for Julia have made it to the website yet. One is from May and one is from September. It is a bit discouraging, but also understandable since neither were suitable for merge immediately.

4 Likes

The changes have been in the in-development docs since the moment the PRs were merged, e.g. Punctuation · The Julia Language for the first PR. They were merged post 1.5.0 and 1.6.0 has not been released yet, so they have not been promoted to the v1 URL yet.

11 Likes

Note: the second was not merged, but it is approved. This is a good example how my suggestion would improve things :smiley: