Skip to content
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

Taking Juleps seriously #33239

Closed
c42f opened this issue Sep 12, 2019 · 35 comments
Closed

Taking Juleps seriously #33239

c42f opened this issue Sep 12, 2019 · 35 comments
Labels
julep Julia Enhancement Proposal

Comments

@c42f
Copy link
Member

c42f commented Sep 12, 2019

For a while now I've felt that we could do better in documenting how pieces of the julia system come to be like they are. I do love that Julia runs on a "show me the code" philosophy, but this doesn't scale to big changes and many contributors. Here are some problems I've observed:

  • The outcome of long design discussion threads is rarely summarized, with the initial issue description containing only the starting point. People have to read through the entire discussion to understand the final design.
  • New contributors have little context for the larger design decisions made in the language. Instead, they have to sift a somewhat unstructured pile of github issues, commit messages and code to extract the buried wisdom. This leads to confusion and wasted effort.
  • Core contributors become a bottleneck in sharing design direction. This leads to frustration as less experienced developers become blocked, and can be an inefficient use of core contributor's time. Better design docs provide a scalable one-to-many solution to this issue.
  • Knowledge about design rationale is lost over time as people move on. A curated repository of design documentation helps avoid this problem. For example, Python's PEP documentation is a fantastic resource. Not just to the Python community, but to everyone interested in related problems in programming language design.

In my experience, the Juleps repository has not been successful in stimulating design discussion, and in any case is too heavyweight for initial triage of an idea and for many small changes. In contrast, the main JuliaLang/julia repository issue tracker has been a good venue for discussion, but is a poor place to curate design documentation for the longer term.

I've recently discovered the golang proposals repository, and I'd like to propose that we adopt something similar to their process:

  1. A julep starts life as a short description in JuliaLang/julia issue tracker.
  2. It is discussed with the aim to either accept, decline or ask for a full design document. If accepted or declined, the process stops.
  3. If a full design document is desired, the author writes it in the Juleps repository and works to address any issues brought up in the initial discussion. Feedback continues on the original issue.
  4. Once the comments and revisions wind down there's a final discussion where the proposal is accepted or rejected.

The intent is to keep our low overhead informal process in the github issues, but to preserve the hard work of design in a more digestible form, regardless of whether it is ultimately accepted or rejected.

@c42f c42f added the julep Julia Enhancement Proposal label Sep 12, 2019
@c42f
Copy link
Member Author

c42f commented Sep 13, 2019

One important point I forgot to make which affects everybody:

  • The outcome of long design discussion threads is rarely summarized, with the initial issue description containing only the starting point. People have to read through the entire discussion to understand the final design.

I've added this to the description above.

@c42f
Copy link
Member Author

c42f commented Sep 13, 2019

In keeping with the proposed workflow, I'd like to request triage of this issue.

When considering this, I'd like to point out that this is a problem most keenly felt by people not working on Julia full time and by people who are isolated, geographically or otherwise (I will admit to being self-interested on both these points). To some extent, this makes it an issue about improving diversity and inclusion in the community.

@c42f c42f added the triage This should be discussed on a triage call label Sep 13, 2019
@tkf
Copy link
Member

tkf commented Sep 13, 2019

3. If a full design document is desired, the author writes it in the Juleps repository and works to address any issues brought up in the initial discussion.

Maybe it's also nice to allow using Juleps PR before the full document is explicitly requested? When there is a long design discussion, it's hard to know the current snapshot of the design. Having a draft document co-evolving with discussion would help people to join into the discussion.

Feedback continues on the original issue.

I see the benefit of this; i.e., keep the discussion in one place. But moving to Juleps PR also has a benefit that it'll linearize the discussion and commits in the document.

@c42f
Copy link
Member Author

c42f commented Sep 13, 2019

The problem with having substantive discussion over at the Juleps PR (or places other than the main issue) is that it scatters the discussion in many places, because there could be multiple PRs required to get the design doc in shape.

I think there's also some hesitation about merging a Julep document if it's not perfect / finished / accepted. But in hindsight this is the wrong way to think about it, and it prevents us from merging worthwhile design docmentation which was rejected, but nevertheless highly worthwhile to keep for historical context.

@tkf
Copy link
Member

tkf commented Sep 13, 2019

Yeah, I see the benefit of keeping discussion in one place.

merging worthwhile design docmentation which was rejected

PEP has "Status" meta data which can take a value like "Rejected":

image

(from https://www.python.org/dev/peps/pep-0001/)

Maybe having something similar can be useful.

@StefanKarpinski StefanKarpinski changed the title Julep: juleps Taking Juleps seriously Sep 13, 2019
@oxinabox
Copy link
Contributor

I'm only about 25% joking when I suggest that these should be titled:

  • Taking X seriously.

Consider:

  • Taking Structured Concurrency Seriously
  • Taking File Paths Seriously
  • Taking String Formatting Seriously

These are good names that are clear on what they are about.
We have precident.

My goodness do we ever take vector transposes seriously

@StefanKarpinski
Copy link
Member

I'm all for calling them "Taking X Seriously"—that's why I called this "Taking Juleps Seriously" 😃

@c42f
Copy link
Member Author

c42f commented Sep 14, 2019

Heh, that title change. I almost did it myself yesterday after @oxinabox suggested it on slack.

I do think that "Taking X Seriously" is a reasonable title for issues which are self-consciously about design. But I also think it's reasonable to apply the julep label to any issue which needs it.

More seriously, what do people think about the Go proposal numbering system? They just use the issue number from the main repo, so, eg, documentation associated with this would be 33239_juleps.md in the juleps repo. I think this is reasonable enough: it's simple and gives unique numbers without any extra infrastructure, and implicitly links back to the discussion. The only minor downside is that the numbers are really large but I think we can put up with that.

@tkf
Copy link
Member

tkf commented Sep 14, 2019

More seriously, what do people think about the Go proposal numbering system? They just use the issue number from the main repo, so, eg, documentation associated with this would be 33239_juleps.md in the juleps repo.

What about discussion happening outside JuliaLang/julia? Like JuliaLang/Pkg.jl#1377

@c42f
Copy link
Member Author

c42f commented Sep 14, 2019

Good question. We could add a directory prefix for standard libraries and for the core language? For example

  • Pkg/1337_StorageProtocols.md
  • Julia/33239_Juleps

This would make a lot of sense once all the standard libraries are split out.

@c42f
Copy link
Member Author

c42f commented Sep 14, 2019

A small thought about metadata: I've often thought there's something slightly unfortunate about Juleps being labelled with an "Author" field. The problem is that this doesn't recognize the critical role that the reviewers play in the process. Instead, I suggest that we use an "Editor" field to name the people responsible for pulling the document itself together, recognizing that the editor may not originate the most important ideas, but that it's their job to present them in a digestable format.

@tkf
Copy link
Member

tkf commented Sep 14, 2019

a directory prefix for standard libraries

That sounds good to me. Another very simple-minded solution is to just use JuliaLang/julia for all Julep discussions.

@c42f
Copy link
Member Author

c42f commented Sep 15, 2019

@tkf I think it could be helpful to have a flow diagram to describe the process. But goodness the PEP system seems a bit complicated.

Actually I favor the option of doing stdlib julep discussions on JuliaLang/julia. One of the hard problems is to get the valuable input of knowledgeable end users. Having the discussion on a relatively obscure stdlib issue tracker only makes that harder.

@oxinabox
Copy link
Contributor

If it only concerns 1 stdlib, then it probably isn't worth a julep. Just a plain feature request issue.

@tkf
Copy link
Member

tkf commented Sep 15, 2019

@c42f At the basic level I was just suggesting to add a few metadata. I guess pre-defining all the possible values of Status and transitions is overkill. I agree that PEP looks overly bureaucratic (though maybe they needed it once the community got to a certain size or something).

@c42f
Copy link
Member Author

c42f commented Sep 15, 2019

@oxinabox definitely for small features. I just noticed that JuliaLang/Pkg.jl#1377 an epic design document by Stefan which will probably have a kind of ecosystem-wide effect. So arguably that kind of big issue stuff could be a julep.

@oxinabox
Copy link
Contributor

Yeah, Pkg is probably the exception.

@quinnj
Copy link
Member

quinnj commented Sep 17, 2019

I fully support a better documented process and I think it'd be helpful to hear from core devs what the best process is for proposing changes and getting things reviewed. Having recently gone through #32799, #32448, and with #32706 and #32859 still open, the experience has honestly been pretty frustrating. Proposing/submitting a change to Base now feels like at least a several-week process, with lots of pinging people, bumping PRs, waiting on answers to questions, pinging people privately, and eventually just threatening to merge, which has felt like the only way to actually get people to review. Now I totally understand the constraints on people's time, and I fight with managing that myself with a number of packages. I just think my frustration should still be voiced as I don't think my recent experience is mine alone; I'm not sure what could be done better, but ideally, people aren't unmotivated to contribute to Base due to the burden of the process.

@c42f
Copy link
Member Author

c42f commented Sep 18, 2019

@quinnj I sometimes feel the same uncertainty myself when trying to progress PRs to the runtime. I vastly prefer to have a solid review before merging, but it's also no fun becoming stuck without a clear way forward. One has to really dig deep into the code to understand the larger implications of changes like #32448. Unfortunately there's only a very small handful of people who can review changes like #32448 and the distributed nature of development means that the larger community can't appreciate how busy those people are, nor easily tap into their expertise (by the way, I await your comments on #33120 which I think is the logical end point to the work you've started with Union :-) )

I think part of the problem here is that writing code and reviewing code are rather different mind sets. In any project which wants to scale its community, the core contributors need to spend proportionally more effort on facilitating (reviewing contributions, both in detail and in high level design) and less on writing code themselves. This is quite a shift in focus and may not come naturally. I've been trying to do a bit more reviewing myself lately as I become more familiar with some of these things, but doing a decent job can require really deep knowledge about how the system functions. Maybe several years equivalent full time development on the runtime to be able to answer difficult questions without reading a lot of code.

I think there's a few things which might help. Just throwing these out there for discussion:

  • A clear written set of expectations about how community contribution is expected to work. Such documentation may be viewed as tooling to scale the community. For example
    • When is a review required? I think we should have a standard such that purely internal improvements which have appropriate tests and pass CI can be merged by the author without further review after a few days or so.
    • Which kind of changes should have a design discussion before they are implemented?
    • What expectations may the community have of the triage team?
  • Investment in more technical tooling to help the core team scale with less effort. For example, tools to track and manage stale issues / PRs. The "forgetmenot" label might be the start of an effort to do this, but I think we need something more dynamic. Some github bots perhaps? I don't know!
  • Some dynamic public visibility on the core development priorities (eg, something like a publicly visible trello board?). The intent would be to align the priorities of occasional contributors better with the core team. I do appreciate the core team commenting on discourse about where development is going, but these are necessarily static snapshots.

@timholy
Copy link
Member

timholy commented Sep 18, 2019

I have certainly felt it from both sides. I'm often surprised when a base PR that seems like it should be exciting gets little attention from those suited to review it (e.g., improving time-to-first-plot, #32705). OTOH I am always really, really grateful when others step forward and take some of the review burden off me in areas where I do have expertise (e.g., JuliaImages) and wish it would happen somewhat more often. I wonder if we can start having more of a process of setting up people with domains of expertise to provide a first-pass review.

@KristofferC
Copy link
Member

KristofferC commented Sep 18, 2019

I think what is quite common is that someone reviews a PR, it looks pretty good but perhaps a few things could be improved. The reviewer will give these potential improvements in the review and then leave the PR. The author is then left in a state where he/she is unsure about the state of the PR:

  • Can it be merged and the comments addressed at a later state?
  • Are the comments so important that it blocks the PR?
  • Are the questions posed in the review just questions and answering them is enough, or do I need to wait for a comment to my answer?

At this point, the reviewer might feel that he/she has done its job and might not return to the PR for a long time, while the author is still left wondering.

I, therefore, think that when reviewing and having comments that are such that the reviewer would block the PR from getting merged until they are fixed, the reviewer needs to properly indicate that. There is the "Changes requested" flag for reviews we could use for this or a label or something. A review that goes through without the "Changes requested" is considered ok to merge, even though the comments indicate that things could potentially be improved.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Sep 18, 2019

I always try to identity what the next actionable step to more a PR forward is and who is expected to take that action. Often people leave things in a state of (a) something has to be done and (b) it's unclear who is able/expected to do that thing. Without these both being clear a PR inevitably stalls.

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Sep 18, 2019

For example, in #32705 I have no idea what the next step is or who would take it. Not to pick on @timholy obviously, but since that PR was brought up as an exciting one that has stalled.

@tkf
Copy link
Member

tkf commented Sep 18, 2019

Isn't the discussion a bit diverged from the original topic (Julep) which is more about designing process? (Or is it about getting a better picture about the overall development process?)

  • Investment in more technical tooling to help the core team scale with less effort. [...] Some github bots perhaps?

Having said that, I cannot help pointing out that increasing discoverability of PRs and issues may help decreasing frustrations. For example, it'd be nice if non-member/contributor can specify or suggest labels. I've seen GitHub bots that help doing that.

@c42f
Copy link
Member Author

c42f commented Sep 19, 2019

@KristofferC great points; the solution is exactly as Stefan says — reviewers should be clear about what action should be taken and by whom. I think it's helpful to distinguish between comments which are broadly in three categories:

  • side notes for context (no action required)
  • cleanup which is "nice to have" but not blocking
  • problems which must be resolved before merging

Doing this in a clear way is hard. Lately I've been trying to writing a summary for the review which highlights the difference between optional cleanup and required changes, and to make sure to ask the author explicitly to act on any required changes.

There is the "Changes requested" flag for reviews we could use for this or a label or something

On a technical level that would be sensible and systematic. Unfortunately from a UX perspective, I often don't like using the "changes requested" flag just because it puts that big red cross on the PR and this feels like an unfortunate "welcome" to people who have just put their code out there, regardless of the nice things I might say in the review text. Especially for inexperienced contributors who are more likely to have changes requested and (potentially?) more likely to be sensitive to criticism.

@tkf I think this discussion about the process of how best to do review is very healthy and rather related so I'm extremely happy to see it happening here. I'd gladly update the description to make the aim for this discussion a "Contributor's Guide" or some such thing.

@oxinabox
Copy link
Contributor

I often don't like using the "changes requested" flag just because it puts that big red cross on the PR and this feels like an unfortunate "welcome" to people who have just put their code out there, regardless of the nice things I might say in the review text.

This is less true now, GitHub changed the icon.

@c42f
Copy link
Member Author

c42f commented Sep 19, 2019

Oh good! Seems to be isaacs/github#990 which was rolled out in January.

@timholy
Copy link
Member

timholy commented Sep 19, 2019

For example, in #32705 I have no idea what the next step is or who would take it. Not to pick on @timholy obviously, but since that PR was brought up as an exciting one that has stalled.

Well, I had put on a "review requested" flagging Jeff in particular---he and I had a brief chat at JuliaCon where he raised some valid-sounding concerns. But I agree that may not have been apparent. I just added some comments that should clarify the key decision a bit.

Unfortunately there's only a very small handful of people who can review changes like #32448

This, I think, is the fundamental problem. There's a tension between "it would be faster to do it myself" and "I will mentor someone through an area that's new to them, and hope that grows the community of expertise." I face that conundrum every day myself, and I am sure my own decisions don't always end up on the side of that boundary that others might wish for.

So that raises a thought: could we establish a bit more of a formal barter system? "I will be happy to help on X if I can get help on Y." Many of favorite projects in the Julia world have been collaborative, but when you're working on "deep" parts of the system (like @quinnj, @c42f) it can sometimes be hard to find a colleague, and rarely can that colleague benefit from your contributions in the way that you can benefit from theirs. But perhaps a deal where you say "I'll do initial triage on all issues & PRs with the tag XYZ for the next 3 months if you help me through PR ABC" might be a way of setting this up?

@StefanKarpinski
Copy link
Member

It's kind of unclear at this point what is to be triaged here...

@StefanKarpinski
Copy link
Member

Some comments from triage: we think we should retire the Juleps repo since that seems to be where proposals go to die. Instead, what seems to work is PRs that have a good mix of reasoning, documentation, and implementation. If people don't want to make an implementation, then a well-reasoned proposal would be the best way to start. More process doesn't seem like it would help make sure that things actually make progress.

@tkf
Copy link
Member

tkf commented Oct 4, 2019

Could you explain how to solve the four bullet points in the OP? Or triage treated them as non-issues?

@c42f
Copy link
Member Author

c42f commented Oct 4, 2019

Lightweight process is great! But it seems my original point has been lost somewhat.

I claim that

  1. It's important to summarize the outcome of important design discussions in an accessible way. Especially as the team grows and changes.
  2. We do this unevenly.

How can we get better at making such design documentation? An issue or PR to JuliaLang/julia is a great starting point for a discussion but the final design may diverge substantially from the OP.

If there's a desire to close the juleps repo we need somewhere else to put this. Maybe it's the devdocs? An advantage of the devdocs over a PEP-like process is that it's living documentation rather than a snapshot. (Talking devdocs, #29527 is somewhat related.)

@tkf
Copy link
Member

tkf commented Nov 1, 2019

Sometimes it is rather disappointing to see the triage misses the important points in the discussion. Please don't take this as me blaming that the people involved in the triage. It is totally understandable that it is hard to pick up the main points to be discussed from a long thread like this. I think I've seen similar examples, not just this one.

Maybe this is where a lightweight systematic approach can help? For example, instead of marking an issue/PR with the triage tag, core devs can use a tag (say) summary requested for a long discussion (especially when it is unclear what to be discussed). Then the author of the issue/PR (or somebody else interested in the discussion) can write up a short summary of the discussion so far. Ideally it should list a few questions and points that need to be decided. I think it would help the triage to focus on important points. It also plays a somewhat overlapping role with Julep in that it provides the latest snapshot of the proposal.

@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Jan 30, 2020
@johnnychen94
Copy link
Member

johnnychen94 commented Feb 11, 2020

Investment in more technical tooling to help the core team scale with less effort. [...] Some github bots perhaps?

Having said that, I cannot help pointing out that increasing discoverability of PRs and issues may help decreasing frustrations. For example, it'd be nice if non-member/contributor can specify or suggest labels. I've seen GitHub bots that help doing that.

FYI, Found a live demo at actions/virtual-environments that might help

@StefanKarpinski
Copy link
Member

I don't know that there's anything actionable on the Julia repo here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
julep Julia Enhancement Proposal
Projects
None yet
Development

No branches or pull requests

8 participants