-
Notifications
You must be signed in to change notification settings - Fork 94
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] Make Amber header in generated Bash files configurable #344
Comments
I think that you can go with a PR with the CLI parameter to not print it. Consider also that Amber is not stable so we can change the Bash output as we are still in alpha so the issue you are facing it will be always there. PS: the next alpha will have breaking changes as example |
I’d just add (at least for now) |
Thanks.
That sounds good to me. In fact, that's already what I do today. In addition to the existing standard Amber header, I use That's also the reason why, long term, I think template approach may be the best. This way I could get rid of Having said that, I also prefer and appreciate KISS approach and if you think something like that doesn't belong in Amber core, I'm also fine with simple Do you plan to make the change yourself? Otherwise I can do it (but it will take a while since I need to set up dev environment, etc and probably won't be able to do it before the weekend). Thanks. |
You can do a PR we are working on various stuff right now that have more priority. |
thats a rather non significant issue for now, but you can make a pr if you want tho. id suggest making it kind of like fastfetch allows customizing outputs (just that module idea, no need to create a config file for that) |
Yeah let's keep it simple. The proposed flag should be sufficient for now. It should be easy enough. If you need any help - let me know! You can reach me here under this Issue, on Discord or via Email 🙌 |
Description
Today, header written by Amber compiler in the generated Bash files looks something like this:
I think it's a good idea to, by default, include date when the file has been generated, but this is also problematic in some cases.
One of those cases is trying to determine if any changes have been made to the Amber source files and if the Bash files need to be re-generated.
Background, Context, Use Case
For reproducibility, transparency and other reasons, I check generated bash files into a git repository. I also have a CI check which runs "Verify Generated Bash Files Are Up To Date" step which verifies that all the generated files are up to date.
This of course doesn't work so well with dynamic header which is different on each compile run (I have some bash hacks to work around it and remove this problematic line, but it's not ideal).
Proposed Change / Implementation
I think in an ideal world, we would make the header fully end user configurable via CLI parameter and also support templates notation.
For example, something along the lines of:
amber --output-header "# Written in Amber\n version ${amber_version}\n date generated: ${date_generated}\n content sha512checksum: ${raw_content_sha512_checksum)" input.ab output.ab
Another (perhaps short term and simpler option) would be to add a new
--output-header-skip-date
flag which makes it skip inclusion of the script generation date / compiler run line in the output file.Please let me know what other people think.
I'm happy to propose the actual change myself, but I would like to get some consensus on the approach first.
It's also worth noting that I'm not a Rust expert so implementing the more complex header option would take me longer than the second "exclude date CLI flag" approach.
Thanks.
Related PRs, Issues
Links
amber/src/compiler.rs
Line 168 in 03c6be2
The text was updated successfully, but these errors were encountered: