search

exercism / legacy-docs

Other
84 stars 55 forks source link

implement-an-exercise-from-specification: Remove avoiding duplicate work section. #108

Closed Insti closed 6 years ago

Insti commented 6 years ago

The Avoiding Duplicate Work section encourages people to create empty WIP pull requests.

But:

So:

Remove this section from the common documentation, if tracks think that it is important to have WIP commits they can add them to their local track documentation.

petertseng commented 6 years ago

I understand that we shouldn't draw conclusions from two isolated incidents, but I will share them in case other incidents indicate a trend.

In both, the empty commit resulting from the WIP PR has caused confusion for a reviewer of the PR. It could be the case that the empty commits are unintuitive, or that the reviewer is not familiar with the practice.

The selection of anecdotes provided by this comment is biased, because no attempt was made to find instances where the WIP commit avoided a conflict.

Insti commented 6 years ago

no attempt was made to find instances where the WIP commit avoided a conflict.

This is a difficult, if not impossible task.

If, once this PR is merged, we are met with a flood of conflicting PRs we can address that issue.

But I very strongly believe that will not happen.

NobbZ commented 6 years ago

I am strongly against a simple empty WIP-commit with an empty PR.

I do not have anything against WIP-PRs in general. But over the last month I got 2 PRs on erlang erlang which started out with empty commit, both without any (meaningfull) PR description, I was happy they mentioned at least the exercises name in the subject…

I prefer if the first commit of a WIP-PR does at least contain the files that make off the exercise (readme, example, tests, stub, etc; the files that are crafted in the process of implementing may be empty for now). Also I really appreciate if in the PR description a rough estimate is given about when we have to expect the first reviewable item, such that I can integrate it better into my daily routine and plan ahead.

I already stated this in the erlang readme, that I want to have this estimate when “obviously unfinished code” is submitted. I'm thinking about hardenforcing this rule with a quote and close, but I do not want to scare the few contributors away that are left…

Insti commented 6 years ago

This appears to be the only place in the Exercism documentation where creating a WIP commit is mentioned. Source Github search

So I guess it's good that people are reading the documentation, but they'll presumably still do that even without this text.

rpottsoh commented 6 years ago

Is it just OK to remove that section of the doc just because we don't like WIPs? Would it be preferable to suggest opening an issue in the track's repo to announce that work has started on porting an exercise or to perhaps refer the contributor to the documentation provided in the particular track they are looking to contribute to? Just thinking out load again.

Insti commented 6 years ago

Would it be preferable to suggest opening an issue in the track's repo to announce that work has started on porting an exercise

No. Tracks should be able to give their own guidance on issues and pull requests. Having such instructions in the common documentation leads to tracks having their process dictated to them rather than controlling their own.

refer the contributor to the documentation provided in the particular track they are looking to contribute to?

I believe this is already present in the document.

rpottsoh commented 6 years ago

Safe to merge or is the PR in some ways dependent on feedback @Insti is looking for in exercism/ruby#762 (comment)?

kytrinyx commented 6 years ago

I'm fine with merging this. I introduced this idea because I was seeing a enough people do the same work at the same time, that it seemed like a good approach. I use WIP PRs at work, and have done in several jobs—very successfully. These are typically about getting early feedback, though, since we don't have random contributors picking up work without mentioning it to anyone at work.

Insti commented 6 years ago

Safe to merge or is the PR in some ways dependent on feedback @Insti is looking for in exercism/ruby#762 (comment)?

This PR can be merged. It is unrelated to that one.