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

Protocol for dealing with outdated pull requests? #148

Open
zkamvar opened this issue Feb 22, 2016 · 3 comments
Open

Protocol for dealing with outdated pull requests? #148

zkamvar opened this issue Feb 22, 2016 · 3 comments

Comments

@zkamvar
Copy link
Contributor

zkamvar commented Feb 22, 2016

This is a question mainly for @hlapp, but as shown in #146, a special case arises when a user attempts to submit a vignette, but it fails because errors in other vignettes due to package updates. Even if these errors are corrected on the master branch, the tests will continue to fail because their branch is an outdated version.

Since these failures would be ideally rare (as their origin are package changes), is there a specific protocol that we can adopt to mitigate this kind of situation? Good examples to try this on: #132 and #143 .

@hlapp
Copy link
Member

hlapp commented Feb 22, 2016

This is the stuff that in theory shouldn't be happening if you are branching off of a master branch that passes tests. If this is becoming a problem, perhaps we need to start locking down the Docker container version being used for integration testing to a particular build via its SHA1.

For more advanced users (such as you @zkamvar 😁) this is more a minor annoyance than a real issue because rebasing against master once it contains the fixes to other packages should readily solve it. But I suppose we don't want to introduce rebasing into our Git workflow instructions.

Also, this is another reason why branches that are open for a long time are not a great idea. Unfortunately, our instructions right now encourage this inadvertently, through the notion of more or less fully developing a vignette before you submit it for a PR. I'm not really sure what the best approach to this is - each alternative has its drawbacks, and the drawbacks are exacerbated for less technical contributors.

@zkamvar
Copy link
Contributor Author

zkamvar commented Feb 22, 2016

This is the stuff that in theory shouldn't be happening if you are branching off of a master branch that passes tests.

In theory is the operative word here 😁

I suppose we don't want to introduce rebasing into our Git workflow instructions.

Given @smanel's reaction to the verbosity of the CWG2R vignette, I would agree.

Also, this is another reason why branches that are open for a long time are not a great idea. Unfortunately, our instructions right now encourage this inadvertently, through the notion of more or less fully developing a vignette before you submit it for a PR. I'm not really sure what the best approach to this is - each alternative has its drawbacks, and the drawbacks are exacerbated for less technical contributors.

Even if the contributor submits a vignette early, the PR will still suffer the same issues if the docker environment changes. One possible way to alleviate this is to encourage users to develop their vignette fully on their own, and then add it to their fork when they are ready (which is how the git2r vignette is structured in a way). One good thing is the fact that github now allows drag and drop into repositories, which would make the submission process a lot easier.

@hlapp
Copy link
Member

hlapp commented Feb 23, 2016

One possible way to alleviate this is to encourage users to develop their vignette fully on their own, and then add it to their fork when they are ready

That's a good point. For a software repo this would make no sense, but indeed in our case that one vignette would depend on, call, or inherit from another will be exceedingly rare or a complete non-issue. You are right to point out that this opens up workflows that would never be considered viable in a software project.

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

No branches or pull requests

2 participants