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

Add highlighting to Primer JSON snippets #547

Merged
merged 1 commit into from Nov 21, 2019
Merged

Add highlighting to Primer JSON snippets #547

merged 1 commit into from Nov 21, 2019

Conversation

lucperkins
Copy link
Contributor

This PR makes the CE primer a tiny bit prettier via syntax highlighting.

Signed-off-by: lucperkins <lucperkins@gmail.com>
@duglin
Copy link
Collaborator

duglin commented Nov 20, 2019

Aside from some indenting difference, what's changed? I'm not see it

@lucperkins
Copy link
Contributor Author

@duglin The JSON snippets now have syntax highlighting, which IMO is an important readability improvement

@duglin
Copy link
Collaborator

duglin commented Nov 20, 2019

It is browser specific thing? Or a plugin? I don't see anything special:
image

@lucperkins
Copy link
Contributor Author

lucperkins commented Nov 20, 2019

@duglin Syntax highlighting is observed throughout the other documents in the spec. This PR is meant only to standardize the primer.

@duglin
Copy link
Collaborator

duglin commented Nov 20, 2019

I'm not against adding the json tags, I'm just curious to see where it would show up since I've never seen it taken into account in a browser

@lucperkins
Copy link
Contributor Author

Without highlighting:

Screen Shot 2019-11-20 at 10 05 54 AM

With highlighting:

Screen Shot 2019-11-20 at 10 06 33 AM

AFAIK it isn't browser specific. Again, I'm only enforcing standardization across the code samples in the spec.

@duglin
Copy link
Collaborator

duglin commented Nov 21, 2019

LGTM
can I get one more LGTM?

Copy link

@Vlaaaaaaad Vlaaaaaaad left a comment

Choose a reason for hiding this comment

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

LGTM! Prettier stuff is always a good thing

@duglin duglin merged commit 248320c into cloudevents:master Nov 21, 2019
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.

None yet

3 participants