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

wip: add Ruby project management lesson #27128

Merged
merged 25 commits into from
May 23, 2024

Conversation

scheals
Copy link
Contributor

@scheals scheals commented Jan 14, 2024

Because

The Ruby course has not covered how to structure your projects or what gems and Bundler are. Our hand was somewhat forced by the change in default Ruby extension for VSCode getting deprecated and Ruby LSP becoming the new default. It expects a Gemfile.lock file to run and will pester the user for what it should do when it doesn't see it. Additionally, it reads RuboCop off the Gemfile (Gemfile.lock? not sure) as well, so to enable linting and formatting features, the learners also need to know how to use Bundler.

This PR

Issue

Closes #27026

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: Ruby Involves the Ruby course label Jan 14, 2024
@scheals
Copy link
Contributor Author

scheals commented Jan 14, 2024

@JoshDevHub Rough, rough draft here. Don't be afraid to pull any punches. I still have to figure out the knowledge checks and additional resources.

@JoshDevHub
Copy link
Contributor

@scheals Awesome I'll have a look.

Also going to cc: @ChargrilledChook because I believe he's interested in having a look at this work as well.

@JoshDevHub JoshDevHub self-assigned this Jan 14, 2024
@JoshDevHub JoshDevHub self-requested a review January 14, 2024 19:46
Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool stuff. It'll be awesome to have this in.

Have a few comments and nits here and there. Also make sure to cleanup the complaints that the new Markdown Linter has.

Main concerns for me are:

  1. What is the larger plan around introducing Ruby-LSP? You seem to reference something in this content that I currently can't find.
  2. How much do we want to go into talking about libraries and managing gems? I think it's very important to introduce because learners will be using RSpec, Ruby-LSP, the debugger, and Rubocop throughout the Ruby course. And that stuff can be very mysterious if they know literally nothing about what's going on with bundler and RubyGems. But I also think we can keep the introduction to this content a bit simpler than what you have here.

But yeah let me know what you think! And let me know if you have any questions/concerns.

@scheals
Copy link
Contributor Author

scheals commented Jan 17, 2024

Main concerns for me are:

  1. What is the larger plan around introducing Ruby-LSP? You seem to reference something in this content that I currently can't find.
  2. How much do we want to go into talking about libraries and managing gems? I think it's very important to introduce because learners will be using RSpec, Ruby-LSP, the debugger, and Rubocop throughout the Ruby course. And that stuff can be very mysterious if they know literally nothing about what's going on with bundler and RubyGems. But I also think we can keep the introduction to this content a bit simpler than what you have here.

As for 1., the Installing Ruby lesson needs a pointer "Hey, for now, choose No whenever it asks for what to do with the Gemfile and ignore any further errors, here's how it should look like if it works correctly: ". Then, the debugging lesson needs to change its suggestion for the extension and I'd go over debugging with both Ruby-LSP and rdbg while recommending rdbg (it's just nicer right now).

2. - which parts would you cut down on? Cutting out semver is fine, perhaps we can ignore what is in the Gemfile and Gemfile.lock and just describe them instead of diving into their parts. Learners at least need to know what RubyGems and Bundler are, how to install gems, how to make sure the project runs with the versions (Ruby, gems) they want.

@scheals
Copy link
Contributor Author

scheals commented Jan 19, 2024

Since I had to update the Debugging lesson's assignment, I decided to give it a very, very basic touch up so learners don't get confused. a170f5d.

I'll get all the linting in place at the very end. It might be good to investigate issues with pry since users are having issues with binding.pry and rdbg debugger seems like.

Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Just one comment about the debug section.

I'll try to get back to you soon about the gem versions stuff.

ruby/introduction/installing_ruby.md Outdated Show resolved Hide resolved
ruby/basic_ruby/debugging.md Outdated Show resolved Hide resolved
@scheals
Copy link
Contributor Author

scheals commented Jan 21, 2024

Took the liberty to change one additional resource and introduce a introductory talk by one of debug's maintainers.

@scheals
Copy link
Contributor Author

scheals commented Jan 25, 2024

Since the lesson overview promises that the learner will get to know what bundle init does, I added a short comment together with a comment for bundle add since it also runs bundle install 90ba6de.

@scheals
Copy link
Contributor Author

scheals commented Feb 4, 2024

Added knowledge checks, decided on the additional resources and did some cleaning up there and there to satisfy the linter. We have some loose ends in the conversations but after we tie them up the lesson should be ready to go when the Rubocop one also gets written.

EDIT: Just now remembered that I probably should not be rebasing/amending/squashing stuff while still working on the PR. Apologies.

@JoshDevHub
Copy link
Contributor

Sorry for my sluggishness on reviewing this. I'll get my thoughts in on the current state of it by the end of the week.

@scheals
Copy link
Contributor Author

scheals commented Feb 6, 2024

Sorry for my sluggishness on reviewing this. I'll get my thoughts in on the current state of it by the end of the week.

No worries. There was still work that I had to do and couldn't do it earlier.

Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this.

I think I've softened a bit on semver and am okay with it being here. It doesn't take up that much space and your explanations are good.

I've included a round of super small grammar and punctuation nits to fix, but I don't have any large points of feedback. I'll probably ask another Ruby/Rails maintainer to look over this content just because it is a significant change, but then I think we'll be fine to merge.

I think it's probably a good idea to hold off on actually adding it to the curriculum until I can get the rubocop stuff done. I'm still working on that, but hopefully will have some work to show soon.

2.5.4
```

<span id="gemfile">There's not much in those but as you can see,</span> the `Gemfile` has information on where to get the gems from and what gems are required. The `"~> 1.1"` is something called a version constraint, particularly one called pessimistic constraint. It relies on semantic versioning - basically, the first number is the major version, second is the minor version and a third, if it exists is the patch number. Major versions can break things from previous versions - think changing method names for example. Minor versions can add and change things but they can't break anything. Patches happen when you introduce bug fixes that don't break anything. So, if people behind a gem maintain it in line with semantic versioning, you can rely on this pessimistic constraint never letting your project have a gem version that could potentially break your app - it is equivalent to `gem "colorize", ">= 1.1", "<2.0"`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<span id="gemfile">There's not much in those but as you can see,</span> the `Gemfile` has information on where to get the gems from and what gems are required. The `"~> 1.1"` is something called a version constraint, particularly one called pessimistic constraint. It relies on semantic versioning - basically, the first number is the major version, second is the minor version and a third, if it exists is the patch number. Major versions can break things from previous versions - think changing method names for example. Minor versions can add and change things but they can't break anything. Patches happen when you introduce bug fixes that don't break anything. So, if people behind a gem maintain it in line with semantic versioning, you can rely on this pessimistic constraint never letting your project have a gem version that could potentially break your app - it is equivalent to `gem "colorize", ">= 1.1", "<2.0"`.
<span id="gemfile">There's not much in those but as you can see,</span> the `Gemfile` has information on where to get the gems from and what gems are required. The `"~> 1.1"` is something called a version constraint, particularly one called pessimistic constraint. It relies on semantic versioning - basically, the first number is the major version, second is the minor version and a third, if it exists, is the patch number. Major versions can break things from previous versions - think changing method names for example. Minor versions can add and change things but they can't break anything. Patches happen when you introduce bug fixes that don't break anything. So, if people behind a gem maintain it in line with semantic versioning, you can rely on this pessimistic constraint never letting your project have a gem version that could potentially break your app - it is equivalent to `gem "colorize", ">= 1.1", "<2.0"`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be a good comma to add too? 🤔

@scheals
Copy link
Contributor Author

scheals commented Feb 6, 2024

O-K, all the suggestions committed, lint problem fixed, another comma to think about aand that's it. Let me know if you want me to squash the commits. And yeah, I'd not add the lesson until the Rubocop one also is ready for release. If you need any help or would appreciate eyes on it, let me know. I'd love to help :D

Copy link
Member

@KevinMulhern KevinMulhern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice work with this @scheals, I think this is going to fill a very big hole in the Ruby course 🚀

Only major (nitpicky) gripe is usage of foo and bar in the examples lol. They tend to be overused in programming tutorials and can be hard to relate to.

We actually went out of our way to avoid using them when rewriting the basic ruby section 😆. Is there anything else we could use?

ruby/introduction/installing_ruby.md Outdated Show resolved Hide resolved
@ChargrilledChook ChargrilledChook self-requested a review February 8, 2024 23:13
@scheals
Copy link
Contributor Author

scheals commented Feb 10, 2024

There's a merge conflict to solve, some linter stuff to fix afterwards (I think) but that'll be for tomorrow. Content changed, swapped, upgraded overall, made better thanks to y'all. Some threads still need to be discussed.

@scheals scheals requested a review from JoshDevHub February 10, 2024 22:09
@JoshDevHub
Copy link
Contributor

This got merged #27936 (which is fine because I see how it's immediately confusing)

But it's also caused a merge conflict here. Are you okay to fix the conflict? @scheals ?

@scheals
Copy link
Contributor Author

scheals commented May 7, 2024

@JoshDevHub
Yeah all good I'll fix that.

scheals and others added 23 commits May 7, 2024 16:36
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Josh Smith <[email protected]>
Co-authored-by: Kevin <[email protected]>
@JoshDevHub JoshDevHub merged commit c08e90a into TheOdinProject:main May 23, 2024
2 of 3 checks passed
@scheals scheals deleted the feat/bundler branch May 23, 2024 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: Ruby Involves the Ruby course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugging: Change suggested Ruby extension to Ruby-LSP and more
3 participants