-
Notifications
You must be signed in to change notification settings - Fork 5
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
Use tagged version of focus-shift #176
Conversation
This indicates more in the face which version of focus-shift we are using. This also upgrade `focus-shift` with the following change: - Ignore non-rendered elements We could think about using `nix` to pull the dependency, but as this may not be updated that much, and as we have only this dependency, keep it simple for now?
controller/dune
Outdated
@@ -4,7 +4,7 @@ | |||
(Changelog.html as Changelog.html) | |||
(gui/reset.css as static/reset.css) | |||
(gui/style.css as static/style.css) | |||
(gui/vendor/focus-shift.js as static/vendor/focus-shift.js) | |||
(gui/vendor/focus-shift-1-0-0.js as static/vendor/focus-shift.js) |
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.
Is there an issue with using the version number in the filename, 1.0.0
?
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.
Do you mean focus-shift-1.0.0.js
for example? I think this sould work as well.
I was not certain it was a good idea, because it looks like an extension, but in the other hand, it gets further away from the version.
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.
Yes, exactly. I think this is the conventional way, and therefore more easily recognized as a version number.
We use this same naming pattern for the ISOs here:
Lines 115 to 120 in 5f6bfb6
+ lib.optionalString buildLive '' | |
ln -s ${components.live}/iso/${components.safeProductName}-live-${components.version}.iso $out/${components.safeProductName}-live-${components.version}.iso | |
'' | |
# Installer ISO image | |
+ lib.optionalString buildInstaller '' | |
ln -s ${components.installer}/iso/${components.safeProductName}-installer-${components.version}.iso $out/${components.safeProductName}-installer-${components.version}.iso |
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 indicates more in the face which version of focus-shift we are using.
This also upgrade
focus-shift
with the following change:We could think about using
nix
to pull the dependency, but as this may not be updated that much, and as we have only this dependency, keep it simple for now?