balena-io-modules / balena-procbots

Process bots used for automating the development and deployment CI pipeline
https://balena-io-modules.github.io/balena-procbots/
Apache License 2.0
7 stars 3 forks source link

Automated generation of build and documentation files #128

Open CameronDiver opened 7 years ago

CameronDiver commented 7 years ago

As we store build objects in the repo along with documentation and of course code, code reviews can get quite difficult when all of the auto-generated files get in the way. If we instead generated these objects post-review, it would mean that the review could focus on the code itself and not have to wade through other files.

Something like

lurch commented 7 years ago

Isn't that exactly what https://github.com/resin-io-modules/resin-procbots/pull/123/commits/c6e171b0e01b29063dc0f46cb474f25b36ca9168 helps to mitigate?

CameronDiver commented 7 years ago

Yeah, but we could have auto-generation and "hiding" for want of a better word bundled together, to reduce the manual tasks that must be done before merging a PR.

lurch commented 7 years ago

On Etcher, we similarly have some files that are autogenerated from others (SASS/CSS files, Docker templates, etc.), with both the source and autogenerated files being stored in the repo. A long time ago I asked @jviotti if it would make sense to only have the source files stored in the repo, with the autogenerated files only created at build / install time, and he replied that he preferred seeing the autogenerated files in the GH diff too, as it allowed him to double-check that the tools that created those files were always doing "the right thing".

jviotti commented 7 years ago

Yeah, I don't really trust pre-processors, after some incidents from our CoffeeScript days, specially if we're not locking down the version of the transpiler.

I wonder if we can somehow get the best of both worlds: have ProcBots auto-generate the build files, but still provide a way for us to double check the result if we need to do so.

lekkas commented 7 years ago

This is a recurring theme and so far the winning argument is the predictability one mentioned by @jviotti (and also @Page- in the past); for interpreted languages, we get to have git history of the exact reversion/codebase that runs in a production setting.

I couldn't agree more that having diffs of autogenerated files is really annoying, but imo suppressing them reduces the pain by a lot (to the point I don't matter having suppressed diffs around, which I can easily skip)

Page- commented 7 years ago

Personally I dislike committing built files on every PR because it makes rebasing and conflict resolution a nightmare, at the same time I don't mind either way for tagged/released versions as it does give that historical view without causing the same pain