-
Notifications
You must be signed in to change notification settings - Fork 28
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
Refactor into using minijinja
templates instead of axohtml
#526
Conversation
559d84c
to
c4c4e36
Compare
minijinja
templates instead of axohtml
c4c4e36
to
1fcc167
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.
A few nits about unwraps, but looks good. Only reviewed the rust, didn't review the templates (seems impractical to try to do).
Pushed up some resolutions of most of my comments. I ended up doing a quick pass over all the I added a couple TODOs for genuine questions I don't want to unilaterally make a call on, that should be resolved before merging. |
/// A map from TargetTriples to Installers that support that platform | ||
/// | ||
/// In theory this should be BTreeMap or IndexMap but something in the pipeline from here to | ||
/// jinja seems to be forcing a sorting so it's deterministic..? Can't find docs for this. | ||
type Platforms = HashMap<TargetTriple, Vec<InstallerIdx>>; |
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.
SEMI-TODO
Also flattens the DownloableFiles type to make it cleaner
Refactors the code to use minijinja instead of axohtml.