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

Feature Request: Update and unify CONTRIBUTING.md and the wiki #4328

Open
3 of 7 tasks
scheals opened this issue Jan 10, 2024 · 2 comments
Open
3 of 7 tasks

Feature Request: Update and unify CONTRIBUTING.md and the wiki #4328

scheals opened this issue Jan 10, 2024 · 2 comments
Labels
Status: Needs Review This issue/PR needs an initial or additional review

Comments

@scheals
Copy link
Contributor

scheals commented Jan 10, 2024

Checks

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this issue follows the Feature Request: brief description of feature request format, e.g. Feature Request: Add a dark mode to the website
  • Would you like to work on this issue?

Description of the Feature Request

Currently, there are two versions of CONTRIBUTING.MD:

The wiki has its own Contributing Guide section which has links to an archived discord channel as a place to seek help with contributing. There's more improvements to be made in my mind, like changing the line:

Install the app on your local machine.

to:

Clone the repo onto your local machine.

There's some consistency issues with using punctuation in the list (some end with, some don't). This is what I'd like to do:

  1. Make sure the .github version of CONTRIBUTING.MD is up-to-date with this repo's.
  2. Remove the repo-specific CONTRIBUTING.MD and let the .github one be the one source of truth.
  3. Remove the mention of running tests from CONTRIBUTING.MD and instead rely on contributors following the Wiki for steps they need to do.
  4. Update various dead links, inconsistent punctuation/inclusion of $ in commands, change all rails commands to bin/rails, use the bin/test script for running tests - especially because that script runs erblint but the instructions fail to mention it needs to be run currently, bin/dev for running the app, perhaps look into improving bin/setup script so the contributor can just run that instead of going step-by-step with installing the gems, JS dependencies, creating the DB, running the tests before starting work, seeding & populating the DB.
  5. Include the wiki article on squashing commits in the Contributing Guide section of the wiki.
  6. Perhaps include the link to the link section of Working with the Curriculum Seeds as one of the steps/in the introduction of Contributing Guide wiki page so every bit of potentially required information is there.

Acceptance criteria

  • Update .github's CONTRIBUTING.MD with the changes in this repo's CONTRIBUTING.md
  • Remove this repo's CONTRIBUTING.MD
  • Remove the mention of running tests from CONTRIBUTING.MD
  • Refresh and improve current instructions in the wiki

Additional Comments

No response

@scheals scheals added the Status: Needs Review This issue/PR needs an initial or additional review label Jan 10, 2024
@JoshDevHub
Copy link
Contributor

JoshDevHub commented Apr 13, 2024

Looks like there was a regression of #4323 brought on by #4478 -- the CONTRIBUTING.md for this repo now says to run yarn test again, which should absolutely change. Maybe there can be a new page on the wiki about commands to run before submitting a PR?

The team settled on having separate CONTRIBUTING docs for handling repo specific concerns, so I wouldn't want to consolidate everything into the parent CONTRIBUTING doc. But other than that all of your ideas seem good here. Ditching the $ in the bash commands is a great idea to give a better copy/paste experience.

You still interested in making a PR (or PRs) for this? @scheals

Glad to answer any questions you have or give guidance on things if you want.

@scheals
Copy link
Contributor Author

scheals commented Apr 13, 2024

Hey @JoshDevHub, I am still interested albeit I am going to focus on the RuboCop lesson at the moment. After I'm done with that I am going to have to set up TOP locally once again from scratch, so then my experience will be fresh.

If someone gets to this before I can (which, if everything goes according to the plan should be around start of may) they should go ahead.

For sure whoever wants can change the testing bit right now could do so, that seems like a quick and important fix. Same with removing $s.

I wonder why the current Gemfile setting for Ruby version has not stopped joel from discord from running stuff. I seem to recall that I was trying to run someone's Rails app recently and I was stopped in my tracks because of a Gemfile-defined Ruby version being different from the one I had enabled (and, I didn't have the target Ruby version at all, in fact). Bundler docs seem to suggest that the current way to link .ruby-version with Gemfile is:

ruby file: ".ruby-version"

Aaand actually, scratch that, I just googled around and supposedly Heroku does not support this and the way it is implemented here should work. So what am I missing? bin/test does use bundle exec @_@

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review This issue/PR needs an initial or additional review
Projects
Status: 📋 Backlog / Ideas
Development

No branches or pull requests

2 participants