Skip to content

ts-migration/valid-expect-in-promise #382

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
Merged
merged 3 commits into from
Aug 11, 2019
Merged

ts-migration/valid-expect-in-promise #382

merged 3 commits into from
Aug 11, 2019

Conversation

@G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Aug 11, 2019

This'll probably conflict a little when #376 is merged, but should be fine.

@G-Rath G-Rath requested a review from SimenB August 11, 2019 20:08
@G-Rath
Copy link
Collaborator Author

G-Rath commented Aug 11, 2019 •
edited
Loading

Ugh WebStorm gets confused when you switch from js -> ts, b/c it marks the js as a generated file, so it told me getStringValue wasn't being used, when its needed by no-large-snapshots 😂

@SimenB I think maybe merging this one ahead of #376 might be the best path, due to no-large-snapshots removing getStringValue.

SimenB
SimenB reviewed Aug 11, 2019
View reviewed changes
src/rules/__tests__/valid-expect-in-promise.test.ts
});
`,
],
invalid: [
Copy link
Member

@SimenB SimenB Aug 11, 2019

Choose a reason for hiding this comment

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

these are just moved right? no actual change?

Copy link
Collaborator Author

@G-Rath G-Rath Aug 11, 2019

Choose a reason for hiding this comment

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

Yup - every other test has valid at the top, & I think the consistency is nice.

@G-Rath
Copy link
Collaborator Author

G-Rath commented Aug 11, 2019

Want me to handle the conflict?

@SimenB
Copy link
Member

SimenB commented Aug 11, 2019

yes please 🙂

@G-Rath G-Rath force-pushed the ts-migration/valid-expect-in-promise branch 2 times, most recently from c6dc65b to fc3be49 Compare August 11, 2019 21:58
@SimenB
Copy link
Member

SimenB commented Aug 11, 2019

This is great! (too bad git gets so confused by renames :( reviewing locally works way better)

@SimenB SimenB merged commit 6a75f24 into master Aug 11, 2019
@SimenB SimenB deleted the ts-migration/valid-expect-in-promise branch August 11, 2019 22:04
@SimenB
Copy link
Member

SimenB commented Aug 12, 2019

🎉 This PR is included in version 22.15.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SimenB SimenB SimenB left review comments

Assignees

No one assigned

Labels

released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@G-Rath @SimenB