Freecodecamp: Squashing and merging PRs

[caravinci]

As per discussions on the core channel, I'm creating this issue so we can document what sort of procedure we should establish for some PRs that go stale on account of their contributor abandoning them after being unable to squash multiple commits into one.

Though it is our belief as a community we should instil the good habit of squashing commits for the sake of repo sanity, some of the contributions are being lost because of enforcing this requirement too strictly. Thus, we've talked about adding an unspoken deadline for squashing before we use Github's squash+merge feature (it's important not to make it too public as not to make some contributors rely entirely on us to do their squashing for them).

Thoughts? @FreeCodeCamp/issue-moderators

[Greenheart]

This is a good idea!

From discussions in the chat yesterday:

If let's say a squash is needed before we can merge, many contributors might feel like their work isn't good enough when all we really ask for is for some finishing touches. So instead of asking them to fix the squash on their own, why don't we reach out to them and offer to talk them through the last steps on a voice/video call. That way, contributors will hopefully feel more welcome - and might contribute even more!

As @atjonathan mentioned, a problem is that many of FCC's core contributors are very busy and might not have time to mentor others.

However, I'd like to think of these outreaches as 15 minute investments that potentially could make more contributors feel welcome in our community - something that'd greatly benefit us in the long run and from my own experience (mentoring people on coding challenges) often is well worth the time to getting in touch with new contributors!

As for the window before an auto-squash is done, I think ~2 weeks is reasonable.

[caravinci]

That sounds good to me.
Anyone else cares to chip in? This would be a change in policy that would influence all of the mods, so it's best if everyone has their say, or at least express their acknowledgement.

[erictleung]

I agree that we should still expect people to squash their commits, but if that is all that is left to do, we can auto-squash it if time passes 👍 I'd be willing to help out.

[ghost]

:+1:

[caravinci]

So PRs stuck at squashing can be mergsquashed after completing two weeks of reaching that state. Ideally, one of us would still try to guide the contributor on how to squash properly.

Also, I've just thought... there's bound to be some who'd repeatedly not squash their PRs. How do we proceed in that case? I don't think rejecting their contribution would be the right approach, but squashing should be encouraged. Ideas?

[dhcodes]

I'm fine with this. How do I squash the commits for them?

[Greenheart]

I've thought about another alternative. Maybe we could make use of the
combined squash-n-merge feature by default? I know, it won't teach
contributors as much but it would ease the burden on those of us who keep
track of the PRs and manage the autosquashing as well as on the
contributors who would have one less step and get a easier workflow. My
reasoning: why do something manually when you could automate the process
and focus on other things?

Thoughts? :)

söndag 2 oktober 2016 skrev Justin Richardsson [email protected]:

So PRs stuck at squashing can be mergsquashed after completing two weeks
of reaching that state. Ideally, one of us would still try to guide the
contributor on how to squash properly.

Also, I've just thought... there's bound to be some who'd repeatedly not
squash their PRs. How do we proceed in that case? I don't think rejecting
their contribution would be the right approach, but squashing should be
encouraged. Ideas?

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/FreeCodeCamp/FreeCodeCamp/issues/11022#issuecomment-250983974,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF12KdVgKumPBxuswKWr6sngCWfUxm0gks5qv-1fgaJpZM4KMB4z
.

[caravinci]

For the same reason we teach proper css while they could just use bootstrap alone.
Automation is dangerous when it implies forgetting or ignoring how it works.
(and in the case of boostrap, dangerous, lol... I know very few people who can actually handle css proficiently these days).

In other repo management systems they don't offer this option, and it'll be useful for our campers to know how to squash. And beyond the knowledge, it's a matter of making sure they understand the importance of repo sanity. The whole check-in|check-out system has been revised a while ago for the model we use of pull-request and quality assessment.

I do agree however, it'd make our job much easier. But to what cost to our students?

[Greenheart]

Ah I see. Good point Justin :)

söndag 2 oktober 2016 skrev Justin Richardsson [email protected]:

For the same reason we teach proper css while they could just use
bootstrap alone.
Automation is dangerous when it implies forgetting or ignoring how it
works.

In other repo management systems they don't offer this option, and it'll
be useful for our campers to know how to squash. And beyond the knowledge,
it's a matter of making sure they understand the importance of repo sanity.
The whole check-in|check-out system has been revised a while ago for the
model we use of pull-request and quality assessment.

I do agree however, it'd make our job much easier. But to what cost to our
students?

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/FreeCodeCamp/FreeCodeCamp/issues/11022#issuecomment-250994074,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AF12KWIcbFiBI08PzmZ0PGUD0Xpg0kuPks5qwBZDgaJpZM4KMB4z
.

[dhcodes]

In keeping with this train of thought, should we also edit branch names before merging for the contributor if it has something like the issue number in it?

[caravinci]

Hmm squashing requires an extra level of understanding of Git... it can be a difficult taks sometimes even for experienced devs. Misnaming branchs/commits, however, is sheer inability to follow instructions. Not sure we need to go that far above and beyond, it should be easy enough for anyone to amend those titles. My 2¢. What doth thou sayeth?

[dhcodes]

I think the principle should ultimately be "if the PR doesn't pass inspection, explain the steps to get it to that point" because regardless of the difficulty of renaming a branch, git has many pitfalls for a new contributor and it can take a lot of trial and error to get a PR to pass.

[caravinci]

So if understand you correctly, there's a way or us to easily rename their branchs/commits from Github? I'm afraid I haven't seen that feature, and I was convinced we'd either need write access to their repo or submit a new PR with our own fork in which we'd rename their commit/branch. If Github has added that capability on merge, I'm all for it.

[dhcodes]

@hallaathrad to my knowledge, when you click the merge button, you can rename the branch and re-write the commit message. I'm not sure of any negative repercussions though.

[caravinci]

Cool! so, does any other involved party want to chip in?

[Bouncey]

I happy with what is being proposed. We should be able to clear more PR's and issues with this

[tannakartikey]

I think it's the best to educate once the PR is received.
Not all project owners are ready to spend a little time teaching new people. The PR would simply be ignored because of little work lacking from the new contributor.
Teaching about finishing PR will encourage contributors positively.

Alternatively, can we use something like Rultor? If the PR is not squashed in the given timeline, we can automatically squash it.

[erictleung]

So I think we all agree on a ~2 week period after a multi-commit, we can go ahead and squash-merge it? Additionally, if a new contributor needs help fixing a commit, whoever comments first on the PR should try to make an effort in guiding the contributor to correct their commits? Shall one of us make this amendment to our guidelines?