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

Do not list files ignored by Sorbet #383

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Do not list files ignored by Sorbet #383

merged 2 commits into from
Jun 6, 2023

Conversation

Morriar
Copy link
Collaborator

@Morriar Morriar commented Jun 6, 2023

We ignore files matching the paths ignored in the sorbet/config and try to reproduce Sorbet behvior.

From Sorbet docs on --ignore:

Ignores input files that contain the given string in their paths (relative to the input path passed to
Sorbet). Strings beginning with / match against the prefix of these relative paths; others are substring
matchs. Matches must be against whole folder and file names, so foo matches /foo/bar.rb and
/bar/foo/baz.rb but not /foo.rb or /foo2/bar.rb.

To produce the same behavior, we match files using File.fnmatch? and we were prefixing and suffixing the paths with **.

When the Sorbet config contains ignored files with extensions like this:

--ignore=foo.rb

This doesn't work as we are trying to match foo.rb with **/foo.rb/**.

To fix this problem, we only append ** to the ignored path IF it does contain a file extension.

Fixes #379.

@Morriar Morriar added the bugfix Fix a bug label Jun 6, 2023
@Morriar Morriar requested a review from a team as a code owner June 6, 2023 16:30
@Morriar Morriar self-assigned this Jun 6, 2023
@Morriar Morriar requested review from egiurleo and KaanOzkan June 6, 2023 16:30
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar force-pushed the at-fix-bump-ignore branch from 604748a to 4c6ddcd Compare June 6, 2023 17:43
@Morriar Morriar requested a review from paracycle June 6, 2023 17:46
@Morriar Morriar force-pushed the at-fix-bump-ignore branch from 4c6ddcd to 787773c Compare June 6, 2023 19:43
We ignore files matching the paths ignored in the `sorbet/config` and try to reproduce Sorbet behvior.

From Sorbet docs on `--ignore`:
> Ignores input files that contain the given string in their paths (relative to the input path passed to
> Sorbet). Strings beginning with / match against the prefix of these relative paths; others are substring
> matchs. Matches must be against whole folder and file names, so `foo` matches `/foo/bar.rb` and
> `/bar/foo/baz.rb` but not `/foo.rb` or `/foo2/bar.rb`.

To produce the same behavior, we match files using `File.fnmatch?` and we were prefixing and suffixing the paths with `**`.

When the Sorbet config contains ignored files with extensions like this:

```
--ignore=foo.rb
```

This doesn't work as we are trying to match `foo.rb` with `**/foo.rb/**`.

To fix this problem, we only append `**` to the ignored path IF it does contain a file extension.

Fixes #379.

Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
@Morriar Morriar force-pushed the at-fix-bump-ignore branch from 787773c to 983c126 Compare June 6, 2023 19:52
@Morriar Morriar merged commit 5ccd6d3 into main Jun 6, 2023
@Morriar Morriar deleted the at-fix-bump-ignore branch June 6, 2023 21:33
@shopify-shipit shopify-shipit bot temporarily deployed to production June 21, 2023 15:04 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spoom bump not ignoring files as expected
3 participants