publiclab / plots2

a collaborative knowledge-exchange platform in Rails; we welcome first-time contributors! :balloon:
https://publiclab.org
GNU General Public License v3.0
956 stars 1.84k forks source link

A few "issues" with first-timers-only issues #2422

Open seafr opened 6 years ago

seafr commented 6 years ago

Some issues with PRs submitted by first-timers

My thoughts / questions

I've noticed that a few PRs by first-timers include the Gemfile.lock file. This seems rather strange to me as Gemfile.lock is listed in .gitignore. Once the contributor commits the changes to Gemfile.lock, they'll have to use git to correct this, and git is not known to be friendly/intuitive (pun not intended). So, should we make mention of this in the PR template?

I've also noticed that some first-timers choose to claim another first-timers-only issue soon after submitting their first PR without waiting for it to be merged. Again, should we make changes in the PR template text to point this out? And to show them where to look next?

Another question the above point raises (to me at least): should we give priority to first-timers' PRs regarding reviews and merges? Anyway, it's safe to bet that a first-timer would feel quite impatient pending acceptance of their first PR :-)

These are just some thoughts on how to make the process smoother and quicker for first-time submitters and reviewers alike. Thank you.

jywarren commented 6 years ago

These are great suggestions and thoughts. I'm going to chime in more soon (traveling right now) but one place we already try to mention the Gemfile.lock is in the Dangerfile; but I think we can do better. Why is the Gemfile.lock getting added in the first place?

https://github.com/publiclab/plots2/blob/efc315d3d7bcc08d167c9f6db7816efd805e66f4/Dangerfile#L9-L11

jywarren commented 6 years ago

should we give priority to first-timers' PRs regarding reviews and merges? Anyway, it's safe to bet that a first-timer would feel quite impatient pending acceptance of their first PR :-)

I definitely agree with this. Maybe we can re-use the first-timers-only label on PRs to indicate this? We might even be able to get Danger to mark them as such if they link to a FTO issue... eventually.

seafr commented 6 years ago

I don't understand either why the Gemfile.lock gets added in the first place. Perhaps some contributors add it explicitly thinking it must be part of the commit. And the Dangerfile message shows up only after they have submitted their PRs, which is a bit too late for a first-timer. I've noticed that a few first-timers have given up at the first sign of struggle, and I guess removing a file from a commit would be a bit of a struggle for them. So maybe we should point out traps to avoid in the FTO step-by-step guide (in the form of steps that should not be taken).

seafr commented 6 years ago

Yes, it's definitely a great idea to label first PRs as such, either with the label you mentioned above or maybe with something like my-first-pr.

jywarren commented 6 years ago

I was thinking of this again and I remembered that I keep a few useful links in my saved replies, especially the one for removing Gemfile.lock!

Hi, this is looking great, but would you mind removing the Gemfile.lock from your PR, as I think it's just a version tweak? https://stackoverflow.com/questions/215718/reset-or-revert-a-specific-file-to-a-specific-revision-using-git

We also have a few helpful guides posted at http://publiclab.org/developers#Resources :

What other common issues do people run into that we could compile resources for? @publiclab/reviewers @publiclab/soc

SidharthBansal commented 6 years ago

One more file test.sqlite. This also creates trouble. I have seen that it has attached itself without on purpose.

SidharthBansal commented 6 years ago

One suggestion I would like to give. Some users find difficult too just jump into the help-wanted issues from the first-timer-issues. So, they try to solve some (not one) on the plots2. So, from my point, we should let newcomers do 2-3 first timers. After that they are comfortable with git and all other stuff, they are ready to jump into the help-wanted issues

mimimalizam commented 6 years ago

Hello 👋

I've noticed that the following gems are not included in the Gemfile.lock.

➜  plots2 git:(tmp-play-branch) bundle install --without production mysql --frozen
You are trying to install in deployment mode after changing
your Gemfile. Run `bundle install` elsewhere and add the
updated Gemfile.lock to version control.

If this is a development machine, remove the /home/vagrant/plots2/Gemfile freeze
by running `bundle install --no-deployment`.

Bundler is unlocking ruby

You have added to the Gemfile:
* unicode-emoji
* omniauth (>= 1.3.1, ~> 1.3)
* omniauth-facebook (~> 3.0)
* omniauth-google-oauth2
* figaro

After noticing this issue, I've learned that it should be excluded since it is in the .gitignore. I tried to remove the Gemfile.lock from git cache (git rm --cached), and then noticed that it is in the repository. So, I'm reasoning that running bundle install locally will update it, which will then be committed by newcomers (e.g. me :))

I can just checkout Gemfile.lock when commiting, but wanted to learn why listed gems are missing in the Gemfile.lock. It will help me to get familiar with your workflow. Tnx 🙇

Also, thanks for the awesome description for issues intended for newcomers. 🍬 👏

grvsachdeva commented 6 years ago

hi @mimimalizam , Welcome to PublicLab community !!!

Regarding Gemfile.lock, we have added it in .gitignore https://github.com/publiclab/plots2/blob/master/.gitignore#L19 but at the time of committing it's not ignored. We would see through it ( @jywarren @publiclab/reviewers any idea of this behavior). We update our Gemfile.lock quarterly https://github.com/publiclab/plots2/pull/1471 so that's why you have seen a message about the new additions to the gemfile. It would be updated shortly, for now, please ignore it. Thanks for reporting. Also, if you are looking for a new issue, please see https://publiclab.github.io/community-toolbox/#r=all and if you can't find there, ping here, we would help you to find one. Thanks :smile:

grvsachdeva commented 6 years ago

@jywarren I think we should update our Gemfile.lock each time new additions to Gemfile is done.What do you think?