Skip to content
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

Issue #75 - Improved processing of non published assets, including versioning #84

Open
wants to merge 1 commit into
base: release/1.7
Choose a base branch
from

Conversation

willprice76
Copy link
Contributor

No description provided.

* @return {@code true} if the specified URL refers to static content within the webapp, {@code false} otherwise.
* @param url a {@link java.lang.String} object.
*/
boolean isNonPublishedAsset(String url);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need default implementation that always returns false to not to break compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would compatibility be broken exactly? As far as I can see there would only be a problem in the situation where the HTML files were published from the CMS and you had put a _version.json file on the web app filesystem or dxa.assets.version property in dxa.properties - which would be a non-sensical situation, but still one which you would resolve by simply removing the file/property

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@willprice76 all these comments in all the PRs from are now for internal usage, so don't worry and ignore all of them :) We cannot merge all your PRs to 1.7 release on github, and we also have internal conventions on how to merge PRs from GitHub (happens through stash quality gate).

Regarding the comment itself: we have never defined what is really public API for DXA, so it doesn't hurt to add a default implementation to also support the case if anyone extended localization implementation or implemented their own localization.

rpannekoek pushed a commit that referenced this pull request Jan 30, 2018
…010 to develop

* commit '137e519db5c2d6eec107e7d0ec749f0823e9b51b':
  TSI-3011 Typo fix (TSI-3010)
  TSI-3011 Review fix (TSI-3010)
  TSI-3011 Review fix (TSI-3010)
  TSI-3011 Added data model extensions to public API (TSI-3010)
  TSI-3010 Added advanced way to save type information without additional changes from CM side (TSI-3011)
  TSI-3010 Moved Conditions to webapp & added support of any classes in extension data (TSI-3011)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants