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

Fix some bugs #404

Merged
merged 7 commits into from
Mar 6, 2023
Merged

Fix some bugs #404

merged 7 commits into from
Mar 6, 2023

Conversation

jpinz
Copy link
Member

@jpinz jpinz commented Mar 6, 2023

  • Readd RemoveSeparatedSectionMutator to the list of defaults, but remove it specifically for NPM.
  • Fix issues where mutations would be just a separator. EX: pkg:npm/q would make pkg:npm/. or pkg:npm/typescript could make pkg:npm/-
  • Fix bug with scoped npm packages with the prefix mutator would add before the @ resulting in mutations like pkg:npm/-%40types/node when that isn't possible. It now adds after the @ but before the namespace begins, resulting in pkg:npm/%40-types/node

@jpinz jpinz added the bug Something isn't working label Mar 6, 2023
@jpinz jpinz requested review from gfs, pmalmsten and brbayes-msft March 6, 2023 20:19
@jpinz jpinz self-assigned this Mar 6, 2023
Copy link
Contributor

@gfs gfs left a comment

Choose a reason for hiding this comment

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

One comment. Probably fine either way but something to consider.

src/oss-find-squats-lib/Mutators/IMutator.cs Outdated Show resolved Hide resolved
@jpinz jpinz requested a review from gfs March 6, 2023 21:19
Copy link
Contributor

@gfs gfs left a comment

Choose a reason for hiding this comment

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

One new comment on using Convert.ToChar vs just taking the first character via indexing. I'm not totally familiar with Convert.ToChar

src/oss-find-squats-lib/Mutators/IMutator.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@gfs gfs left a comment

Choose a reason for hiding this comment

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

LGTM

@jpinz jpinz merged commit 730b367 into main Mar 6, 2023
@jpinz jpinz deleted the jupinzer/readd_default_mutators branch March 6, 2023 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants