-
Notifications
You must be signed in to change notification settings - Fork 336
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
chore: remove build steps from readme and reference build.yml #9558
Conversation
Signed-off-by: Matt Krick <[email protected]>
@rafaelromcar-parabol whatcha think? safe to reference the build yml instead of writing it out? |
Sorry man. I'm into other things and didn't have time to this. TBH, I do not like this change. I've used that readme many times to quickly copy and past the commands to build the application locally into a Docker container. I know @Dschoordsch has built it to, but I'm not sure if using this readme. I agree that removing the readme makes it better in some way, but harder in other. And I like to be able to copy and past if I need to quickly build a Docker image because something is wrong. I'll like to get @Dschoordsch and @dbumblis-parabol take on this. |
Ah, I guess I don't know enough about how yall are using this! |
@mattkrick I only build the docker image locally if I want to test/debug something related to the docker image. |
Just to be clear, all the commands to build the application are NOT just like that on the build CI. You have to be able to understand how to run the databases locally using Docker. Not that hard, but just to be clear about it. |
I think building locally isn't something we should support . We can't guarantee the same architecture, it's easy to have modified node_modules, different env vars, etc. If we want to build an ad-hoc image, we can do that using a GH runner using a workflow: right now we've got 2 sources of truth on how to build the app: 1 yml, 1 readme. my goal here is to reduce maintenance overhead. maintaining a readme sucks, so readmes should only cover large fundamental ideas & leave the specifics for the code. whatcha think? |
It is true that now with the workflow we can build images on demand from branches and just pull them from our private dev repository. I didn't know about Act. I still think this readme is useful but I understand and agree with what you say about only one source of truth. Let's get rid of it. If we feel the need in the future, we can always add something back. |
Signed-off-by: Matt Krick <[email protected]>
Description
The best readme is self-documenting code!
Our build.yml script already outlines exactly how to build our app, so our readme should instruct folks to read the yml instead of creating a second source of truth.
I also renamed
.yaml
to.yml
so all our yml files have the same extension.