chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.8k stars 421 forks source link

Simplify .future file format #7121

Closed ben-albrecht closed 7 years ago

ben-albrecht commented 7 years ago

The process of submitting Futures predates Chapel's usage of GitHub issues.

Futures are a useful tool in Chapel development. However, there are a couple of downsides to the current process when combined with GitHub issues:

The current .future format could benefit from relying more on GitHub issues. I propose that we simplify the .future file format to mandatorily include a title using the existing format of: category: title, and an issue # on its own line. This leaves the extended description as optional.

Preferably, the future title would match the issue title, and the future category would match one of the issue labels.

So a .future's contents could be:

feature request: Simplify .future file format
#7121

or:

feature request: Simplify .future file format
#7121

The process of submitting Futures predates Chapel's usage of GitHub issues.

...

This would also enable us to automatically track futures from issues, as requested in #4964.

vasslitvinov commented 7 years ago

The "category: summary" line seems helpful for example for a quick triage of the future.

Do we want to require "Issue" before the issue number, like "Issue #7121" for added robustness?

ben-albrecht commented 7 years ago

The "category: summary" line seems helpful for example for a quick triage of the future.

I agree, but this format isn't strictly enforced already. I think continuing the format of "category: summary" or even "category: issue-title" would be a good practice, but I don't think it should be mandatory.

for added robustness?

I'm not sure what you mean by added robustness. The syntax "#7121" already makes it clear to me that it's referencing a GitHub issue or pull request, and it's also easily parsable.

bradcray commented 7 years ago

The "category: summary" line seems helpful for example for a quick triage of the future.

I agree, but this format isn't strictly enforced already.

Just because something isn't enforced doesn't mean it isn't mandatory... I think we should continue to make it mandatory. Adding it when creating a .future is low-overhead compared to getting from an issue # to the webpage describing the issue (using a URL would reduce the overhead, but personally I'd still make the traditional header mandatory even if we switched to using a GitHub URL).

vasslitvinov commented 7 years ago

The syntax "#7121" already makes it clear to me that it's referencing a GitHub issue or pull request, and it's also easily parsable.

OK, makes sense.

ben-albrecht commented 7 years ago

Just because something isn't enforced doesn't mean it isn't mandatory

Fair point. So to be clear, you'd prefer a format like this, correct?

category: title
#issue-number

<optional description>

Would we prefer/restrict the issue-title match the .future title, with exception of the category prefix?

bradcray commented 7 years ago

Yeah, though I don't feel strongly about where the issue number should appear (where you propose is fine; as would putting it on the first line or further down in the file).

I think we'd prefer for the .future title to match the issue title (and for the category to match a tag on the issue), but I don't know how strict we need/want to be — if the issue # is required that makes the linkage explicit.

ben-albrecht commented 7 years ago

@bradcray - works for me.

I've updated the original proposal based on feedback from @bradcray && @vasslitvinov

lydia-duncan commented 7 years ago

Would "allow issues to track futures" allow issues to close when the .future file gets deleted because it has been resolved?

ben-albrecht commented 7 years ago

Would "allow issues to track futures" allow issues to close when the .future file gets deleted because it has been resolved?

Not sure, but that'd be cool.

Without that functionality, the person deleting the future(s) could copy the issue number(s) into the PR description, and the issue(s) would be closed upon merging, e.g.

Closes #7121 (or) Resolves #7121

bradcray commented 7 years ago

Would "allow issues to track futures" allow issues to close when the .future file gets deleted because it has been resolved?

Personally, I'd be inclined not to have issues be closed when .future files are deleted since I'd like a human to be in the loop for that (where BenA's suggestion could be one way of involving a human). For example, I could imagine an issue that had multiple futures hanging off of it where retiring one future wouldn't be sufficient to resolve the future as a whole.

lydia-duncan commented 7 years ago

Good point! I was more thinking of the simple case where we forgot about the issue involved but deleted the future just fine

psahabu commented 7 years ago

@ben-albrecht, would you want to take this on this week?

ben-albrecht commented 7 years ago

This has been announced as the new format to the team.

bradcray commented 7 years ago

Updating the text describing futures in doc/rst/developer/bestPractices/TestSystem.rst might help future generations of Chapel future-writers.

ben-albrecht commented 7 years ago

@bradcray - Good call!

bradcray commented 6 years ago

Having lived with this decision for a few months now, I find that it hasn't been as easy/attractive as I'd expected it to be in the abstract. That's not to say that I'm dead-set against it, but wanted to rationalize why I don't use it more and see what others are doing in practice.

Specifically, I find that I almost always start by creating the future test first because most of the time I run into issues when I'm writing code, so tend to already have the code which demonstrates the issue. This leads me to create a .future file and, due to history, I write some notes about the issue in the .future file.

Next, I create a PR for the future in order to get it into the code base. I do this prior to filing an issue because our issue format is nicest when you use the form to set up hyperlinks to point to the future and to the PR that merged the issue. So I need to create a PR to get a number for the issue prior to creating the issue (or really, finalizing it).

Then I go and create the issue. By this point, the future is already merged or ready to merge, so it feels like a hassle to go back and edit the .future file again, commit and push it.

I realize that with more passes or a maybe a willingness to have all of these things happening simultaneously, I could take this approach, but generally speaking it starts to feel a bit fussy and I find I like visiting each component once linearly where the natural order for me is most often: (1) write the test, (2) open the PR, (3) create the issue.

(Of course, not having any scripting to match issues to futures and vice-versa doesn't help encourage the new format much either... Not that I necessarily feel like I'm suffering due to the lack of scripting)

What are others doing and how are they finding it?

lydia-duncan commented 6 years ago

I tend to wait to create the .future file until I understand the problem a bit more/have determined whether I can solve it quickly or whether it will actually need a .future file after all. I also do the same thing with the issue, so when I've reached the point that I'm ready to do either I find it easy to create the issue, then make the .future file, then open the PR.

But I've mostly operated in two modes: responding to issues others open, working through problems I'm tracking elsewhere out of habit/ease of modification.

vasslitvinov commented 6 years ago

I agree it is a hassle to have to put the issue# into the .future.

I alternate between merging a separate PR replacing a "TBD" in the .future with the issue number and creating the issue some time between creating a PR with the .future and merging it.

When I create an issue, I can put in the name of the test file into it even before the test appears on master.

I would welcome a scheme that does not require an issue# in the .future, or possibly does not require an issue at all if the .future is self-explanatory.

ben-albrecht commented 6 years ago

I tend to wait to create the .future file until I understand the problem a bit more/have determined whether I can solve it quickly or whether it will actually need a .future file after all.

I also follow this workflow, mostly due to following the path of least resistance. I find it less burdensome to create an issue with a code snippet copied in and some details about how to reproduce the error over writing a test and reproducing the environment in the test system, followed by opening a PR. Maybe the original proposal here assumed too much about the typical issue-filing process of developers.

I would welcome a scheme that does not require an issue# in the .future, or possibly does not require an issue at all if the .future is self-explanatory.

I strongly disagree with the latter statement here. I think all futures should map to an issue that can be searched and updated easily. Putting issue numbers in the .future was the mechanism proposed here, but I'm open to reconsider if we reach a consensus on a superior mechanism.