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.

Sign up for GitHub

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

Jump to bottom

Improve some XML test strings #183

Closed
wants to merge 2 commits into from
Closed

Improve some XML test strings #183

wants to merge 2 commits into from

Conversation

brodybits
Copy link
Member

resolves #128 (followup to PR #123 - reformat test code using Prettier)

@brodybits brodybits marked this pull request as ready for review January 25, 2021 21:04
@brodybits brodybits requested a review from karfau January 25, 2021 21:04
karfau
karfau approved these changes Jan 25, 2021
View reviewed changes
Copy link
Member

@karfau karfau left a comment

Choose a reason for hiding this comment

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

LGTM
(Even though I'm not aware why this way of concatenating strings is any different from the other way.)

@brodybits
Copy link
Member Author

Better alignment. Yeah maybe not really worth it?

@karfau
Copy link
Member

karfau commented Jan 27, 2021

Better alignment. Yeah maybe not really worth it?

Did you already handle all of them?
Since you invested the time we can also land it. But I'm not sure we will be able to keep it this way in the future...
Your choice :)

@karfau
Copy link
Member

karfau commented Apr 17, 2021

Since you didn't land it: Couldn't/Shouldn't we nowadays just use template strings everywhere, consistently?

@brodybits brodybits marked this pull request as draft April 18, 2021 01:50
@brodybits
Copy link
Member Author

Shouldn't we nowadays just use template strings everywhere, consistently?

Makes sense to me, yes.

@brodybits brodybits closed this Apr 18, 2021
@brodybits brodybits deleted the improve-xml-test-strings branch April 18, 2021 01:52
@brodybits brodybits mentioned this pull request Apr 18, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karfau karfau karfau approved these changes

Assignees
No one assigned
Labels
code style testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consistently use template strings for XML snippets
2 participants
@brodybits @karfau