Static view registrations with package root asset specifications #1377
Conversation
This file contains 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
tilgovi
added a commit
to hypothesis/h
that referenced
this pull request
Jul 18, 2014
There are a number of reasons to isolate the static assets into their own directory. - An edge case in Pyramid: Pylons/pyramid#1377 - Make serving them with a reverse proxy safer by ensuring that no code or configuration lives beneath the static directory - Cleanliness To avoid a big move happening shortly after this, also rename some of the asset subdirectories to close #1317. - js -> scripts - css -> stylesheets - lib -> scripts/vendor - lib/images -> images - browser/chrome/js/background.js -> browser/chrome/background.js - browser/chrome/public/js/destroy.js -> browser/chrome/public/destroy.js Finally, the locale directory was totally busted. The annotator.po file included is the defualt locale, and it should have been .pot. Since it includes only the default strings, there's no reason to serve it. I've stripped it and we'll reinstate it correctly later. Close #1311
tilgovi
added a commit
to hypothesis/h
that referenced
this pull request
Jul 18, 2014
There are a number of reasons to isolate the static assets into their own directory. - An edge case in Pyramid: Pylons/pyramid#1377 - Make serving them with a reverse proxy safer by ensuring that no code or configuration lives beneath the static directory - Cleanliness To avoid a big move happening shortly after this, also rename some of the asset subdirectories to close #1317. - js -> scripts - css -> stylesheets - lib -> scripts/vendor - lib/images -> images - browser/chrome/js/background.js -> browser/chrome/background.js - browser/chrome/public/js/destroy.js -> browser/chrome/public/destroy.js Finally, the locale directory was totally busted. The annotator.po file included is the defualt locale, and it should have been .pot. Since it includes only the default strings, there's no reason to serve it. I've stripped it and we'll reinstate it correctly later. Close #1311
|
This fix looks good. Can you push another commit signing the contributors.txt? |
tilgovi
force-pushed
the
package-root-static-view-spec
branch
from
4339608
to
0b0551c
Compare
tilgovi
force-pushed
the
package-root-static-view-spec
branch
from
0b0551c
to
0b0ea0a
Compare
|
@mmerickel rebased, tweaked the changes message, and added myself to contributors. Thanks! |
|
LGTM. |
mmerickel
added a commit
that referenced
this pull request
Nov 13, 2014
Static view registrations with package root asset specifications
|
Thanks again @tilgovi! |
|
Cheers. |
nickstenning
pushed a commit
to hypothesis/browser-extension
that referenced
this pull request
Jul 8, 2016
There are a number of reasons to isolate the static assets into their own directory. - An edge case in Pyramid: Pylons/pyramid#1377 - Make serving them with a reverse proxy safer by ensuring that no code or configuration lives beneath the static directory - Cleanliness To avoid a big move happening shortly after this, also rename some of the asset subdirectories to close #1317. - js -> scripts - css -> stylesheets - lib -> scripts/vendor - lib/images -> images - browser/chrome/js/background.js -> browser/chrome/background.js - browser/chrome/public/js/destroy.js -> browser/chrome/public/destroy.js Finally, the locale directory was totally busted. The annotator.po file included is the defualt locale, and it should have been .pot. Since it includes only the default strings, there's no reason to serve it. I've stripped it and we'll reinstate it correctly later. Close #1311
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I'm not entirely sure this isn't the intended behavior, but I ran into this and couldn't find anyone discussing exactly this in a previous PR or issue.
Rather than bother you all with whether or not to fix/change this, I thought it was a good opportunity to dig in and just see how some of the internals work to try to fix it.
Here's a PR with a failing test in one commit and then a subsequent commit that fixes it and adds a CHANGES comment.
Let me know if I'm mistaken and this is inappropriate or something needs re-working / re-wording since it's my first time contributing to Pyramid and I may have missed some aspect of process / policy.
Thanks!