-
Notifications
You must be signed in to change notification settings - Fork 43
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
Developer Onboarding Simplification #112
Conversation
## Install System-Level Dependencies | ||
|
||
- [rtx](https://github.com/jdx/rtx) | ||
- [ImageMagick](https://imagemagick.org/), used to manipulate images on upload. Currently, there's not a reliable way to manage this with `rtx`, so you'll need to use your distribution package manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor thing — and maybe I am just not the best reviewer here, as I'm on an M1 Mac — but I think this might be a step in the wrong direction, from a documentation perspective. The previous version of this linked to ImageMagick
's site (https://imagemagick.org/script/download.php
) which has relatively comprehensive instructions on installing.
To be clear, they are not great — they make some assumptions about the reader's ability to install the binary — and the system package manager might be better advice, but I feel like it's better to link to something concrete and authoritative than nothing.
In general, I'm not totally on-board with the push of rtx
here. With the caveat, again, that I'm on a Mac, I've had issues with rtx
not being universally better than other management tools (rbenv
handles M1 compilation and openssl
shenanigans better than rtx
does, as of writing).
But, more so, a big selling point of rtx
and asdf
-type "multi-tool managers" is that they operate well with "legacy version files" — .node-version
, .ruby-version
, etc. On a technical level, there's no requirement that anyone use rtx
, it's just the "tool manager" we currently like best.
I think it's fine to point folks at our recommendation — we were doing that before with rbenv
, and I think it's fine to switch that to rtx
for Linux folks (I, personally, still think rbenv
is better for M1 Mac folks, at least, but I am fully neutral as to what we recommend on Linux), I think it's a step backwards to have these instructions written with a specific tool as their focus.
If nothing else, I really don't think we need to repeat rtx
's instructions for setting up tool management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gonna tackle this in chunks (and if I don't get around to responding to all of it in this pass, I'll respond to the rest tomorrow). Firstly though, thank you for the write-up here and for all the thought put into it, I really appreciate it!
I fell back to system package manager for ImageMagick only because the asdf plugin for managing source-based IM installs doesn't support MacOS at all (ARM or Intel), otherwise there's a Linux-friendly solution there. I'm open to the idea of forking and fixing that plugin to support MacOS, and thus removing the system-level dependency entirely here, though I'm less familiar on whether our app code is using a Gem to talk to ImageMagick or if it's calling out to the system using $PATH
. The latter is a great fit for rtx/asdf-based setups; the former I'm not entirely certain about ("in theory" the Gem "just" builds against whatever libraries are in the include paths that pkg-config
, gcc
, etc. find, but the existence of... the entire ecosystem worth of C/C++ package managers and their tangled webs of complexity tells us that "just" is a loaded word here).
For rtx
as the "true path", I'm still poking at the MacOS edges (see PR description) here so I haven't found every wart, but given that rtx
is using ruby-build
under the hood here the same as rbenv
, I'm somewhat surprised to hear you had unique challenges there. I'll hold too many further MacOS-specific thoughts until I've finished the port-over here.
Anyway, I wanted to use rtx
as the "true path" here (for both Linux and MacOS) because of two main factors: 1, that it can pin exact versions (and build them from source if needed) in a way that few cross-platform package managers can (Nix can do it, but speaking from experience rolling out Nix to an org before: no, absolutely not, I do not want to maintain that as the "primary" path), and 2, because it can handle (via asdf
plugins) just about any dev tool under the sun in the same general way, regardless of underlying OS, and it does so performantly. I retained a note in the docs about how one is welcome to not use rtx
, but I generally prefer (so this is subjective and somewhat philosophical) having Exactly One Supported Way To Do It and letting the user explore on their own terms if they for some reason disagree with the choices or can't use the given tool. To me, the existing docs read like a bit of a "choose your own adventure" novel, whereas for expedient external contributor onboarding, I think I prefer "here's the battle-tested path that works in under 20 minutes, if you want to do something else, none of this stuff is proprietary or hidden behind too many layers of paint, go for it".
This philosophy is also the core of why I copied out rtx
's installation instructions into this guide, so I'll reuse that "all in one place, all as one tested set of instructions" rationale here. I should also note that I do more or less the same for Homebrew in the draft of the MacOS docs I have sitting around, so this is probably something we should figure out some agreed philosophy on before long; I'd like to have a common voice between both documents if I can.
Anyway! None of this is set in stone, I appreciate having someone who isn't a Linux daily driver (Mac and I both are) to bounce ideas off of here and to remind me of blind spots, so consider this reply more as providing backstory and context (and asking for more conversation on some areas) than it is necessarily "defending" one way or the other quite yet 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaw6 could you take a look at @klardotsh comment Friday? Thank you so much!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, I mean, evaluating this kind of change is tricky, because I have not actually used the earlier version of this document to stand up a Forem instance on any Linux. And - I imagine - you actually have that experience! If this document would have been more helpful in that effort, I don't want to stand in the way of improvement.
For me, for what it's worth, I don't really think the issue is rtx
, or version managers generally, per se, but that's the concrete detail I was tripping on earlier.
I think the previous version of this having linked to ImageMagick
's install documentation seems like it would be a more useful resource than it would be to remove that link. Describing that there isn't a reliable way to manage this with rtx
doesn't seem like a net improvement over linking to deeper instructions. Since we've disclaimed this with "instructions were written for Debian 12", maybe we could actually include the details on how to install imagemagick
on debian? (Since those should more-or-less work for debian-derived distributions, I think?)
Again, for what it's worth, just, like, as a reader, the version of this doc on main
is organized and coherent in a way that the proposed change is not. It starts with installing pre-requisites, it sometimes recommends version managers, it often links to detailed guides for more details.
As a reader, the proposed change is hard to follow. rtx
is mentioned at the top of "Install System-Level Dependencies", then followed by a bullet-list of somehow vague dependencies — lacking installation instructions or specific version details — then a code block that details how to install rtx
. (As I reader, I would have expected this code block to actually install the preceding list of dependencies.) Later, under "Install Application-Level Dependencies", there's discussion and links for alternatives to Postgres and Redis (this strangely being the first time redis is mentioned in the document, and the first time we've mentioned Postgres directly, rather its "various transitive dependencies").
Maybe we could try to preserve some of the original document's structural organization, even if we are shifting about things like rtx
. That could be something like:
- maintainer sanity disclaimer
- how to install rtx
- how to install dependencies with rtx
- other dependencies not managed by rtx (imagemagick, say)
- how to install forem (yarn install, etc.)
- Possible error messages (this seems useful, is it relevant now?)
(I'm not sure why rtx
wasn't able to install ruby, it looked like it choked on openssl
, which has happened to me a lot and was more-or-less resolved when homebrew and I got out of ruby-build
's way. I didn't dig deeper, I did not realize the ruby plugin was based on ruby-build
, so that discrepancy is interesting, but I can look into it, at some point. It seems like it might be useful to figure that out.)
a326dd2
to
e91aa22
Compare
@jaw6 Thanks for the detailed feedback. I've pushed several things this evening that I think address your feedback and makes the overall flow better for both OSes (while keeping the flows nearly identical for both), and makes my opinionation a bit less strongly enforced in the writing (while still making clear that the docs are Trying To Get Folks To Do Things One-ish Way):
For MacOS I've left the "possible error messages" section in since I see some wild stuff with I'd greatly appreciate a re-review from @jaw6 if time allows. I'd also appreciate a review and end-user walkthrough (async on your own time is great, but I'm around during reasonably-normal PDT working hours this week to hop on a call to debug if something goes sideways) from @maestromac and @mirie (thanks for offering!). I'm not certain these are the exact instructions that will merge, but I think after a few days away from them and a fresh mind, and being able to spin up the MacOS side so quickly with them, I'm feeling reasonably confident in going to bat for this PR as-written now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I had some very minor suggestions
e91aa22
to
e1299d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Barring some auto-start script, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems easier to follow now 👍
This is the documentation part of the bare-metal developer onboarding simplification work I've been tinkering with, which depends on forem/forem#20077.
I have the MacOS equivalent update in progress on my disk - I wanted to have the Linux side rock solid and tested a couple times smoothly before doing the (for me, at least) more complex case, and now need to fix access to my AWS account before I can do the final test pass on that 🤦 So for now, this one's Linux-only. I'll either amend this PR to add the MacOS bits, or open a fast-follow as soon as I'm done spinning up a Mac EC2 instance to test and doing this like committing the Homebrew lock file.
On an AMD 6900HX system with a Debian 12 QEMU VM, I went from "first login to the system" to "example unit tests passing" in less than 20 minutes, using only copy-pasted instructions from this guide and no further Linux or Forem knowledge. This time includes compiling Redis, PostgreSQL, and Ruby all from source via
rtx
, and so will vary system-to-system. To say I'm thrilled with this outcome is an understatement, as our goal was ~30min.The tradeoff here is that I've largely killed off all the "there's more than one way to do it" paths that used to exist in this documentation. I don't completely disavow doing things any other way, but the docs now explicitly tell you you're on your own if, for some reason, any of our opinions don't match your own, and give hints on where to look for tool versions to go fend for yourself.