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

Chore: Fix prettier issues in test files #1710

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Conversation

zoltanbedi
Copy link
Member

@zoltanbedi zoltanbedi commented Oct 10, 2023

Fixes #1709

@zoltanbedi zoltanbedi requested a review from a team as a code owner October 10, 2023 14:10
@zoltanbedi zoltanbedi requested review from gabor and gwdawson and removed request for a team October 10, 2023 14:10
@Antiz96
Copy link

Antiz96 commented Oct 10, 2023

I confirm 54612e6 fixes the issue. Thanks @zoltanbedi!

@zoltanbedi
Copy link
Member Author

Thanks @Antiz96. Is this blocking you right now?

@Antiz96
Copy link

Antiz96 commented Oct 10, 2023

@zoltanbedi Well, I'm at least waiting for this PR to be reviewed/merged (just by conscience) to package the new version on Arch Linux repo.
If there's a plan to release a v4.4.4 including this fix shortly after that, I can wait for it. Otherwise I'm able to simply backport the fixing commit in our v4.4.3 package in the mean time :)

Copy link
Contributor

@gabor gabor left a comment

Choose a reason for hiding this comment

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

everything looks good, but there's that regex-escape thing i describe below, we may want to look at it first.

// Backward compatibility
it('should properly split query in old format', (done) => {
let test_cases = [
{
query: `/alu/./tw-(nyc|que|brx|dwt|brk)-sta_(\w|\d)*-alu-[0-9{2}/`,
expected: ['/alu/', '/tw-(nyc|que|brx|dwt|brk)-sta_(\w|\d)*-alu-[0-9{2}/']
expected: ['/alu/', '/tw-(nyc|que|brx|dwt|brk)-sta_(w|d)*-alu-[0-9{2}/'],
Copy link
Contributor

Choose a reason for hiding this comment

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

\w\d got turned into wd. i think this is a correct prettier-transform, but the original text looks very regex-ish... maybe the "before" situation was already wrong?

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 are right the query to escape w properly needs double backslash.

@zoltanbedi
Copy link
Member Author

@Antiz96 if you are not blocked then we won't cut a release for code formatting. We will do that once we have other things to release as well.

@Antiz96
Copy link

Antiz96 commented Oct 12, 2023

@zoltanbedi Sure, sounds good to me. I'll just backport the fixing commit once this PR is merged :)

@zoltanbedi zoltanbedi requested a review from gabor October 12, 2023 14:53
@zoltanbedi zoltanbedi self-assigned this Oct 12, 2023
Copy link

@gabor-grafana gabor-grafana left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@zoltanbedi zoltanbedi merged commit 43f2c66 into master Oct 13, 2023
1 check passed
@zoltanbedi zoltanbedi deleted the zoltan/prettier-fixes branch October 13, 2023 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

make lint is failing on v4.4.2/v4.4.3
4 participants