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

Custom parameters prefix knotx snippet #419

Conversation

malaskowski
Copy link
Member

Description

Snippet parameters names are too long, because of being HTML5 compliant (as the snippet tag is <script>). Since Knot.x 1.2.1, it is possible to customize snippet tag name (see #385 for more details).
This enchancment is followup of this discusion.

Motivation and Context

  • snippet parameters should enable defining HTML compliant prefixes, but when not necessary should be simplified.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I hereby agree to the terms of the Knot.x Contributor License Agreement.

@malaskowski malaskowski changed the title Feature/custom parameters prefix knotx snippet Custom parameters prefix knotx snippet Apr 26, 2018
Copy link

@pun-ky pun-ky left a comment

Choose a reason for hiding this comment

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

Type parameter was required for script tag. Is it still required when non script tag is used? / knotx:snippet?

@malaskowski
Copy link
Member Author

@pun-ky thanks for this catch, of course it is not required.
I've updated wiki with information about it.
Good catch.

* Default constructor
*/
public SnippetOptions() {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

what about default values ? a good practice is to have private init() method invoked in default constructor and inside jsonobject constructor (before converter). an init method will simply set default values for tag and prefix. that way we avoid potential nullpointers

Copy link
Member Author

Choose a reason for hiding this comment

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

You're right. I first thought that there will be no default values, but this is very good place to have those.

@malaskowski malaskowski merged commit 91818c6 into milestone/backpressure-and-configs May 8, 2018
@malaskowski malaskowski deleted the feature/custom-parameters-prefix-knotx-snippet branch May 8, 2018 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants