Skip to content

Encourage new contributors to start with bugfixes? #2553

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

Closed
misaochan opened this issue Mar 7, 2019 · 12 comments
Closed

Encourage new contributors to start with bugfixes? #2553

misaochan opened this issue Mar 7, 2019 · 12 comments

Comments

@misaochan
Copy link
Member

misaochan commented Mar 7, 2019

Had a conversation with @maskaravivek and @ashishkumar468 the other day - we were wondering if it might be a good idea to recommend new contributors to start with bugfixes, at least for their first few PRs. There are a few reasons for this:

  1. Benefits to the student - I personally think that you learn a lot about a codebase by fixing bugs. It requires you to delve deep into the code and figure out what it's doing and what is wrong with it. By doing so, you also learn about the coding style, structure, etc of the project. Enhancements are more like adding a lot of your own code on top of the existing code - good for coding practice, but not so good for project familiarization. Also, when you work on bugfixes you do not need to wait as long for a community consensus, compared to a debated enhancement.

  2. Benefits to the project - We are trying to prioritize stability in this year, and I think we need to be a bit more careful with adding new features. New features inherently increase complexity and increase surface area for bugs, so it is important that the benefits are worth the added complexity. Additionally, new features written by folks who are not-so-familiar with the codebase often break our guidelines re: code structure, style, etc. It is very time-consuming to keep up with the PRs to check for all of this. Currently our PRs are very backlogged - hopefully things will get better next week once @neslihanturan is back from leave, but it is still a lot to go through.

There is precedent for this - for instant, Mozilla's guidelines for new contributors ONLY involve fixing bugs: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction

If we do carry this out, we need to decide to what extent we want to enforce this. Do we just want to recommend that people start with bugfixes, but leave the decision to them? Or do we want to be stricter about it? How many bugfixes does it take for a contributor to be reasonably familiar with the codebase?

@maskaravivek
Copy link
Member

maskaravivek commented Mar 7, 2019

I agree with @misaochan and I believe it would be beneficial for both the contributors and for the project itself.

Edit: IMO this can be enforced strictly so that all collaborators can follow this consistently. The guideline can be to assign issues to new contributors only with bug/crash/beginner friendly (we can add other labels if needed) labels.

To assign enhancements to a contributor, we can quickly check his past contributions to the projects and assign only if he/she has more than x ( lets say 5) contributions to the project.

If we reach a consensus on this, we can edit the Volunteer wiki to reflect the same.

Also, on a related note, can we start enforcing unit tests for any changes that are being made. With the IEG approved, we are anyways moving to TDD, so it would be great to have other contributors follow the same guidelines. :)

@misaochan
Copy link
Member Author

misaochan commented Mar 7, 2019

@maskaravivek Good thought. I think maybe we can require unit tests for PRs targeting enhancements, but not for PRs targeting bugfixes/beginner-friendly issues? After all, the reason we were hesitant to require unit tests is because we did not want to raise the barrier of entry too much. But if new contributors will only be working on bugfixes and beginner-friendly issues, then I think it should be OK to require them to start writing unit tests once they have moved on to enhancements.

@maskaravivek
Copy link
Member

Yup good point! Lets keep tests mandatory just for feature addition. Also, tk clarify just unit tests would be required and not UI tests, right?

@misaochan
Copy link
Member Author

Yes, no unit tests for UI components. :)

@nicolas-raoul
Copy link
Member

Would this be only for GSoC/Outreachy people, or any GitHub newcomer?
Let's say that in 2017 tech wizard Magnus Manske himself comes and singlehandedly adds perfect 2FA authentication just because he wants that feature. Should we refuse that PR?
My suggestion would be to limit the strict requirement to GSoC/Outreachy people, while having it as just a recommendation for other people.

@misaochan
Copy link
Member Author

Let's say that in 2017 tech wizard Magnus Manske himself comes

Haha, he would need to be a literal wizard to turn back time like that! ;)

I am theoretically OK with restricting the requirement to GSoC/Outreachy applicants (as those form the vast majority of our new PRs anyway), but in practice wouldn't it be a bit difficult to know who the applicants are? Since the contributions would happen long before they officially apply.

@maskaravivek
Copy link
Member

@nicolas-raoul's example of a very reputed developer wanting to contribute is reasonable. But that shouldn't be the norm. It should be an exceptional case where multiple collaborators agree. Some of the factors that may help a collaborator decide can be:

  • volunteer's experience with open source (Github profile can be an indicator)
  • some indication that the volunteer is familiar with Android development

When an experienced developer comes and contributes to the project, it is very lucrative to merge it all but the burden of squashing the bugs comes to the maintainers of the app. And given the fact that the app is open source but still actively maintained just by a handful of developers who are working against a grant deadline, its difficult to be very lenient.

@misaochan
Copy link
Member Author

misaochan commented Mar 13, 2019

Seems like @nicolas-raoul is in agreement? In that case, let's go ahead and create this rule. :) I think it should only affect PRs created AFTER the date that we edit the wiki, i.e. we should accept the PRs that are already up, if they satisfy all the other requirements. Aside from that, if an issue was already assigned to someone before that date, I guess we should allow them to proceed with their PR too?

@nicolas-raoul
Copy link
Member

Sounds worth trying!
If in the future the number of new contributors becomes really too low, then I guess we can revisit this decision :-) In that case we might want to try seasonal restrictions, with stricter rules in the pre-GSoC/Outreachy periods.

@misaochan
Copy link
Member Author

misaochan commented Mar 14, 2019

I have edited the "Volunteers Welcome" section of the wiki. To all contributors, please note that these changes will take effect on unassigned issues starting now. Thanks all! :)

@nicolas-raoul
Copy link
Member

Is this "and" or "or"?

may only be assigned issues with "bug" and "beginner-friendly" labels

Currently there are only 4 issues with "bug" and "beginner-friendly" labels:
https://github.com/commons-app/apps-android-commons/issues?q=label%3Abug+label%3A%22beginner+friendly%22+is%3Aopen

@misaochan
Copy link
Member Author

Sorry, I meant "or" indeed.

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

No branches or pull requests

3 participants