-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Inline iro.js and rangetouch.js #3597
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
b2babd9
inline iro.js and rangetouch.js
w00000dy 5cd8f56
Remove unused feedback function
w00000dy 0a91d60
add onload event to body
w00000dy 1140f5f
Update version to 2312190
w00000dy 201daf8
Remove /iro.js and /rangetouch.js
w00000dy 86d2998
Add reference to old loading of iro.js and rangetouch.js
w00000dy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I would keep this for future reference as a comment.
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.
I see no advantage in keeping that as a comment. If you want to look at the code again in the future, you can simply look at the Git history.
I think that would just make the code less readable and most people would probably wonder what this comment is for when they see it.
And what exactly would you want to leave as a comment? The complete
<script>block or just the feedback function?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.
History is one thing, actual snippet embedded as a comment is another. Snippet can be used immediately by anyone while code from history is only viable option for those that know it was there. Snippet (as a comment) does not reduce readability of the code, even without any actual comment in it, on the contrary IMO it adds value for a possible future when such feature may be re-implemented.
Dynamically loading of JS files may come in handy in the future (for possible extensions) and at least POC code should be kept in place as a guideline for such case. Commented code does not increase binary code size or adds performance penalties.
I am in favour of reducing HTTP requests to ESP but not at the expense of removing knowledge from source files. Otherwise you can strip every comment from all source files.
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.
I added a comment with referencte to how it was done in 0.14.0. What do you think of this?
I think this is better because we don't have this big block of commented out code (so the code looks cleaner) and we still preserve the knowledge.