# Making the review process more pleasant for new contributors

**URL:** <https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604>\
**Category:** General Usage\
**Tags:** github, contributing\
**Created:** [August 8, 2023, 2:00pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604 "2023-08-08T14:00:25Z")\
**Posts on this page:** 20\
**Page:** 2

<div class="post-metadata">

**Author:** ![Sukera](https://avatars.discourse-cdn.com/v4/letter/s/ce7236/32.png) [@Sukera](https://discourse.julialang.org/u/Sukera)\
**Post date:** [August 8, 2023, 9:01pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/21 "2023-08-08T21:01:33Z")

</div>

Well, to me such a response is certainly having the opposite effect…

---

<div class="post-metadata">

**Author:** ![Lilith](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/lilith/32/27492_2.png) [@Lilith](https://discourse.julialang.org/u/Lilith)\
**Post date:** [August 8, 2023, 9:15pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/22 "2023-08-08T21:15:13Z")

</div>

Browsing recent open and closed issues on github, of those that are missing versioninfo in the OP, few would imo be improved by including it. Requiring version info in a template would introduce unnecessary work for issue reporters in some cases and introduce extra noise for maintainers in some cases.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://discourse.julialang.org/u/jules)\
**Post date:** [August 8, 2023, 9:17pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/23 "2023-08-08T21:17:34Z")

</div>

We have a relatively short list of things to check off when opening Makie issues. I’m not sure if it helps that much, often people don’t fill them in correctly. (Or maybe the list could be better)

I think the less critical or complex the surface that a PR or issue touches, the less process you need. We don’t need the same process to apply to docs PRs as to compiler PRs. Somehow our organizational principles should reflect that and keep overhead for the simple stuff low. It should be easy to get docs PRs in and probably a lot of people should be allowed to accept those.

---

<div class="post-metadata">

**Author:** ![adienes](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/adienes/32/37459_2.png) [@adienes](https://discourse.julialang.org/u/adienes)\
**Post date:** [August 8, 2023, 9:22pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/24 "2023-08-08T21:22:24Z")

</div>

GitHub has lots of features that could possibly be useful (or possibly just noise?)

for example, consider the following policy:

- anyone who has authored a commit can “approve” PRs (just an endorsement, not actually making it ready to merge)
- PRs getting some X number of unanimous approvals from community contributors are labeled with “needs maintainer review” or something like that

---

<div class="post-metadata">

**Author:** ![Lilith](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/lilith/32/27492_2.png) [@Lilith](https://discourse.julialang.org/u/Lilith)\
**Post date:** [August 8, 2023, 9:26pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/25 "2023-08-08T21:26:08Z")

</div>

> [@jishnub](#):
>
> I wonder if this might be automated?

> <https://github.com/JuliaCI/Nanosoldier.jl/issues/177>
>
> It would be lovely to trigger nanosoldier to bisect a MWE. For example
> 
> @nanos…oldier bisect
> \`\`\`julia
> using Test
> 
> a = 0:1/24:365 - 1/48 - 0.001
> b = 0:1/24:365 - 1/48
> typeof(a) # StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}
> 
> @test a==b # true
> @test collect(a)==collect(b) # false!!!
> \`\`\`
> 
> Could run that snippet on various Julia versions and search for when \[#50039\](https://github.com/JuliaLang/julia/issues/50039) was introduced, reporting the relevant PR or commit in a comment as well as a link to the full logs which would include the error messages and/or debug output produced form running it on each tested version. It would be helpful performance-wise to cache pre-built versions of Julia on the nanosoldier machines and do bisection in a cache-aware manner. Start and endpoints for the bisection could be optionally provided and default to 1.0.0 and the lastest master.

---

<div class="post-metadata">

**Author:** ![jishnub](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/jishnub/32/33620_2.png) [@jishnub](https://discourse.julialang.org/u/jishnub)\
**Post date:** [August 9, 2023, 4:57am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/26 "2023-08-09T04:57:33Z")

</div>

Apologies, this was not meant sarcastically, nor was I taking a dig at your suggestion. This was merely a lighthearted middle ground, which I didn’t think would be misinterpreted. However, tone is hard to convey.

That being said, a point that I was trying to convey is that — currently, the wall of text appears to be ignored by many, and it merely gets in the way. Users would just delete the text to frame their issue. By the time they are done pasting their output, the suggestion to include the version is long gone. Clippy, on the other hand, exists independent of the text box, and is less likely to be ignored if it emerges occasionally. Then again, I wasn’t suggesting this seriously, and an issue template _also_ solves this issue by discouraging users from deleting text.

Perhaps a more realistic middle ground could be a `reprobot` that tries to see if the text in a newly filed issue contains an `Exception`, in which case it tries to detect the version number and platform from the text. If this fails, it leaves a message for the OP to provide this information. If there is no response for a week, the bot attempts to reproduce the issue on the latest stable version as well as the LTS on tier 1 platforms, and posts whether it succeeds in reproducing the issue. In case an issue is not reproducible and the original poster is missing for months, this makes it easier for maintainers to tag it as stale and close it.

---

<div class="post-metadata">

**Author:** ![carstenbauer](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/carstenbauer/32/4981_2.png) [@carstenbauer](https://discourse.julialang.org/u/carstenbauer)\
**Post date:** [August 9, 2023, 5:58am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/27 "2023-08-09T05:58:42Z")

</div>

> [@Sukera](#):
>
> though I have no idea whether that wasn’t an accidental click and was meant as a ❤ instead. The buttons are too close on mobile.

This!!! I misclicked so many times!

---

<div class="post-metadata">

**Author:** ![carstenbauer](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/carstenbauer/32/4981_2.png) [@carstenbauer](https://discourse.julialang.org/u/carstenbauer)\
**Post date:** [August 9, 2023, 6:07am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/28 "2023-08-09T06:07:11Z")

</div>

> [@jules](#):
>
> It should be easy to get docs PRs in and probably a lot of people should be allowed to accept those.

Agree 100%. I would’ve loved to help merge a bunch of those but don’t have the rights, at least not for Julia proper. I guess that’s also because I never dared to ask over the years because I don’t contribute so much “hard code” to base (I work on packages etc.). Maybe we shouldn’t only give “core developers” merge rights but also “core community members”, who could clearly help with doc PRs? (And I want to say that irrespective of my own case.)

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://discourse.julialang.org/u/jules)\
**Post date:** [August 9, 2023, 6:44am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/29 "2023-08-09T06:44:45Z")

</div>

Maybe, if we don’t want to give merge rights to many people, we could instead have people without merge rights review docs PRs. Not sure if it saves time for a core contributor to hit merge if some other people have already put their “approved” stamp on the PR.

I’m not sure if one can give out partial merge rights, like only for branches with specific tags. I assume not, as then giving the tags would also need specific rights etc.

---

<div class="post-metadata">

**Author:** ![carstenbauer](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/carstenbauer/32/4981_2.png) [@carstenbauer](https://discourse.julialang.org/u/carstenbauer)\
**Post date:** [August 9, 2023, 7:59am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/30 "2023-08-09T07:59:05Z")

</div>

> [@jules](#):
>
> Maybe, if we don’t want to give merge rights to many people, we could instead have people without merge rights review docs PRs. Not sure if it saves time for a core contributor to hit merge if some other people have already put their “approved” stamp on the PR.

I don’t disagree, but:

1. The few times I left my “approved” stamp on a PR, it felt like it was worthless (to the person merging the PR later). Maybe it wasn’t, but at least I didn’t get any feedback whatsoever that it was helpful. So I stopped doing it.
2. Personally, I noticed that I tend to care much more about repositories where I’m a member/maintainer. But maybe that’s just me.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://discourse.julialang.org/u/jules)\
**Post date:** [August 9, 2023, 8:05am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/31 "2023-08-09T08:05:51Z")

</div>

I agree with your points, I was thinking about what options you have when you’re not a member/maintainer, but yeah just putting approval under a PR might not really have any effect. If the maintainer doesn’t trust the reviewer they will check themselves anyway, saving no time. If they trust, then maybe they should have given merge rights to that person already.

With docs, I think one interesting aspect is that in my opinion core maintainers are not necessarily the best people for writing those. Sure, they understand the subject matter the best, but usually that means documentation turns out quite terse and lacking context information that a lot of casual users might not have. On the other hand, some subjects need a very high degree of accuracy when writing docs. But those are probably not things casual contributors would feel qualified to add to anyway, so it might not be a big issue in practice that people add slightly incorrect information about tricky topics.

---

<div class="post-metadata">

**Author:** ![Sukera](https://avatars.discourse-cdn.com/v4/letter/s/ce7236/32.png) [@Sukera](https://discourse.julialang.org/u/Sukera)\
**Post date:** [August 9, 2023, 8:30am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/32 "2023-08-09T08:30:08Z")

</div>

> [@jishnub](#):
>
> Apologies, this was not meant sarcastically, nor was I taking a dig at your suggestion. This was merely a lighthearted middle ground, which I didn’t think would be misinterpreted. However, tone is hard to convey.

All good! Indeed, tone is very hard to convey over text. 🙂

> [@jishnub](#):
>
> Then again, I wasn’t suggesting this seriously, and an issue template _also_ solves this issue by discouraging users from deleting text.

Not only discourage, but they can be built in such a way to keep the suggestive text around! A good example for that is IMO the way the `helix` editor structures their issue reporting. (Please don’t submit issues to helix just to try the flow out 😅 ) Once you [click on the `New Issue` button](https://github.com/helix-editor/helix/issues/new/choose), you get a choice between `Blank Issue` (basically the existing default we’d have), `Bug Report`, and `Enhancement`. Github also has “Don’t see your issue here? Open a blank issue” text below, so even if we don’t add an explicit `Blank Issue` template, it’ll still be there.

Once you [hit `Bug Report`](https://github.com/helix-editor/helix/issues/new?assignees=&labels=C-bug&projects=&template=bug_report.yaml), all the key information is asked for in a form, keeping what is asked around even after the textfields are filled in. I can see this kind of persistent “Hey we need this information” as the Clippy you’d think of, except there’s much less development overhead for maintainers because we don’t then have to maintain a GitHub Bot on top of the repo.

> [@jishnub](#):
>
> the bot attempts to reproduce the issue on the latest stable version as well as the LTS on tier 1 platforms, and posts whether it succeeds in reproducing the issue. In case an issue is not reproducible and the original poster is missing for months, this makes it easier for maintainers to tag it as stale and close it.

Automatically reproducing posted code is usually hard; not sure that’s going to be a viable strategy. I do agree that marking issues as stale would be good though, and it would be even more helpful for PRs that have had failing CI that’s not getting fixed. Closing outdated PRs instead of keeping them around is a good thing.

---

<div class="post-metadata">

**Author:** ![gdalle](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/gdalle/32/224680_2.png) [@gdalle](https://discourse.julialang.org/u/gdalle)\
**Post date:** [August 9, 2023, 11:05am UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/33 "2023-08-09T11:05:39Z")

</div>

I agree that the experience is very pleasant! It seems to come from the experimental “issue forms”. Would a PR be welcome to set up one or two for the Julia repo? It seems a bit meta to open an issue to discuss the shape of issues

> **[Configuring issue templates for your repository - GitHub Docs](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository#creating-issue-forms)**
>
> You can customize the templates that are available for contributors to use when they open new issues in your repository.

---

<div class="post-metadata">

**Author:** ![Lilith](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/lilith/32/27492_2.png) [@Lilith](https://discourse.julialang.org/u/Lilith)\
**Post date:** [August 9, 2023, 6:11pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/34 "2023-08-09T18:11:35Z")

</div>

One barrier to merging documentation PRs is that we define public API by “it’s in the manual”. It would be so nice if we could decouple documentation from public API.

---

<div class="post-metadata">

**Author:** ![gdalle](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/gdalle/32/224680_2.png) [@gdalle](https://discourse.julialang.org/u/gdalle)\
**Post date:** [August 9, 2023, 6:17pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/35 "2023-08-09T18:17:50Z")

</div>

Yeah, this means that a documentation change can be breaking. Which is kind of messed up

---

<div class="post-metadata">

**Author:** ![Sukera](https://avatars.discourse-cdn.com/v4/letter/s/ce7236/32.png) [@Sukera](https://discourse.julialang.org/u/Sukera)\
**Post date:** [August 9, 2023, 6:58pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/36 "2023-08-09T18:58:42Z")

</div>

Unfortunately, describing every bit of behavior of something in formal detail is quite hard 😅 Not to mention checking that the formalized behavior is actually correct & what you want it to be.

Still, documenting it is better than not documenting it!

---

<div class="post-metadata">

**Author:** ![tim.holy](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tim.holy/32/52_2.png) [@tim.holy](https://discourse.julialang.org/u/tim.holy)\
**Post date:** [August 9, 2023, 7:08pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/37 "2023-08-09T19:08:39Z")

</div>

We don’t (deliberately) merge PRs that cause new failures. Most current causes of failures probably passed CI when they were initially submitted, but intermittent issues or hardware changes or subtle interactions with other changes might cause them to (sometimes) fail; if you have enough flaky tests and failure of just one is enough to call the CI run a failure, then CI failures are common.

I’m not saying this is good, but chasing down the causes tends to be hard work and not that many people make this a core mission. If you want to find examples of PRs fixing flaky tests, a subset (probably only a minority) can be found with the “ci” label: [Pull requests · JuliaLang/julia · GitHub](https://github.com/JuliaLang/julia/pulls?q=is%3Apr+is%3Aclosed+label%3Aci+)

Sometimes, though, things get through due to inadequate test coverage and only discovered later.

While I’m not at all confident this is in any way meaningful, I amused myself by comparing Julia and [Python](https://github.com/python/cpython):

- Julia, source files ending in `.jl` under `base/`: ~130K LOC
- Julia, files ending in `.jl` under `test/`: ~100K LOC
- Python, source files as discovered by `flspy=$(find . -name "*.py" ! -name "test_*")`: ~400K LOC
- Python, test files as discovered by `flspy=$(find . -name "test_*.py")`: ~470K LOC

So proportionately we seem to be in roughly the same ballpark, with `ntestLOC ÷ ncodeLOC` slightly lower for Julia than for python. I do think there’s an argument to be made that because Julia code may be more composable, we might need a higher ratio of tests-to-code than Python, and we don’t have that.

---

<div class="post-metadata">

**Author:** ![tim.holy](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/tim.holy/32/52_2.png) [@tim.holy](https://discourse.julialang.org/u/tim.holy)\
**Post date:** [August 9, 2023, 7:17pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/38 "2023-08-09T19:17:16Z")

</div>

FWIW, I do appreciate it when someone else reviews a PR before I get to it. But I doubt I remember to thank the previous reviewer(s) very often. So I see why it’s unrewarding.

A good analogy is, suppose a trainee has a draft of a manuscript but the language in which the manuscript is written is not the native language of the author. It’s really nice if the university’s writing center can take an initial crack at the manuscript, to smooth over some of the language. But that doesn’t mean that I as a scientist won’t find additional issues after the comments from the writing center have been addressed. Indeed, sometimes smoothing out language-specific bumps makes it easier to discover “deeper” problems and might net a larger number of comments than if the writing center had never been engaged.

I would go so far as to say that attracting more initial reviews from the community would be my favorite change to make to how we merge PRs. It would be really nice to have a system where such reviewers can build trust, and where approval by higher trust levels send increasingly-urgent messages to people with merge privileges that this PR is deserving of a final decision, etc. I am not sure whether that’s something we can set up, though.

---

<div class="post-metadata">

**Author:** ![gdalle](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/gdalle/32/224680_2.png) [@gdalle](https://discourse.julialang.org/u/gdalle)\
**Post date:** [August 9, 2023, 8:48pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/39 "2023-08-09T20:48:01Z")

</div>

If you want to upvote a new issue template:

> <https://github.com/JuliaLang/julia/issues/50861>
>
> The current issue template can feel a bit intimidating, and at the same time it …is not always well-followed. GitHub recently introduced \[issue forms\](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository#creating-issue-forms) in beta, and some people on \[Discourse\](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/) feel like these forms could provide a nicer experience for beginners.
> 
> I can draft a PR to get this started if there is enough interest.

---

<div class="post-metadata">

**Author:** ![gdalle](https://sea2.discourse-cdn.com/julialang/user_avatar/discourse.julialang.org/gdalle/32/224680_2.png) [@gdalle](https://discourse.julialang.org/u/gdalle)\
**Post date:** [August 9, 2023, 8:50pm UTC](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/40 "2023-08-09T20:50:29Z")

</div>

> [@tim.holy](#):
>
> It would be really nice to have a system where such reviewers can build trust, and where approval by higher trust levels send increasingly-urgent messages to people with merge privileges that this PR is deserving of a final decision, etc. I am not sure whether that’s something we can set up, though.

I kinda liked the suggestion made by @adienes earlier, maybe GitHub has ways to make our life easier in this regard

> [@Making the review process more pleasant for new contributors](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604/24):
>
> GitHub has lots of features that could possibly be useful (or possibly just noise?) for example, consider the following policy: anyone who has authored a commit can “approve” PRs (just an endorsement, not actually making it ready to merge) PRs getting some X number of unanimous approvals from community contributors are labeled with “needs maintainer review” or something like that

Here are a few GitHub actions that could be useful:

- [Label approved pull requests · Actions · GitHub Marketplace · GitHub](https://github.com/marketplace/actions/label-approved-pull-requests) to prioritize based on number of approving reviews
- [Pull Request Stats · Actions · GitHub Marketplace · GitHub](https://github.com/marketplace/actions/pull-request-stats) to find and evaluate reviewers

[Previous page](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604.md?page=1)

[Next page](https://discourse.julialang.org/t/making-the-review-process-more-pleasant-for-new-contributors/102604.md?page=3)