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

Change the FilterVersions rewriter tests to use the filter_versions! method #269

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

egiurleo
Copy link
Contributor

@egiurleo egiurleo commented Feb 21, 2024

@Morriar pointed out that the Tree#filter_versions! method looks like deadcode because it's not tested anywhere. I've changed the FilterVersions tests to call this method instead of using the underlying API, as I would expect a consumer to do.

Note: this also matches the SortNodes tests

@egiurleo egiurleo requested a review from a team as a code owner February 21, 2024 22:28
@egiurleo egiurleo requested review from st0012 and vinistock February 21, 2024 22:28
@@ -15,7 +15,7 @@ class Buzz; end

tree = Parser.parse_string(rbi)

Rewriters::FilterVersions.filter(tree, Gem::Version.new("0.4.0"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's keep one test using the class method version, as it's also part of the public API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@egiurleo egiurleo force-pushed the emily/filter-versions-test branch from 84561f9 to 55cdd73 Compare February 22, 2024 16:25
@egiurleo egiurleo merged commit fd05e9b into main Feb 23, 2024
8 checks passed
@egiurleo egiurleo deleted the emily/filter-versions-test branch February 23, 2024 17:28
@Morriar Morriar added the chore Chore task label Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chore task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants