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: account for base when suggesting rerun cmd #992

Merged

Conversation

GV14982
Copy link
Contributor

@GV14982 GV14982 commented Oct 28, 2023

PR Checklist

Overview

Add additional filter logic to check if an options entry was excluded by the base, and filter it out of the suggested rerun flags, add new test, update failing test with suggestion from jest

(>^u^)><(^u^)><(^u^<)

Add additional filter logic to check if an options entry was excluded by
the base, and filter it out of the suggested rerun flags
@codecov
Copy link

codecov bot commented Oct 28, 2023

Codecov Report

Merging #992 (9d9f6bd) into main (6c15440) will increase coverage by 0.02%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #992      +/-   ##
==========================================
+ Coverage   94.10%   94.12%   +0.02%     
==========================================
  Files          95       95              
  Lines        5428     5448      +20     
  Branches      431      436       +5     
==========================================
+ Hits         5108     5128      +20     
  Misses        319      319              
  Partials        1        1              
Flag Coverage Δ
create 70.85% <59.52%> (+0.11%) ⬆️
initialize 41.13% <59.52%> (+0.22%) ⬆️
migrate 69.83% <59.52%> (+0.11%) ⬆️
unit 71.58% <100.00%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/create/createRerunSuggestion.ts 100.00% <100.00%> (ø)
src/shared/options/augmentOptionsWithExcludes.ts 73.14% <100.00%> (+1.15%) ⬆️

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@GV14982
Copy link
Contributor Author

GV14982 commented Oct 30, 2023

I am not entirely sure why this is failing compliance checks?

@JoshuaKGoldberg
Copy link
Owner

From the failure linked in the ❌: https://github.com/JoshuaKGoldberg/create-typescript-app/actions/runs/6690991139/job/18177354788?pr=992:

Error: This PR's title should conform to specification at https://conventionalcommits.org/

bug: is a prefix for an issue; fix: would be the right title for a PR. A few linked issues that I hope will eventually make the explanations better:

Copy link
Owner

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Wonderful, thanks! This will make my life a bit easier for #734 😄

Not much to change, the core logic is looking great. Just a few small touchups. What do you think?

cspell.json Outdated Show resolved Hide resolved
src/create/createRerunSuggestion.test.ts Show resolved Hide resolved
src/create/createRerunSuggestion.ts Outdated Show resolved Hide resolved
src/create/createRerunSuggestion.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg added the status: waiting for author Needs an action taken by the original poster label Oct 30, 2023
@GV14982 GV14982 changed the title bug: account for base when suggesting rerun cmd fix: account for base when suggesting rerun cmd Oct 30, 2023
@github-actions github-actions bot removed the status: waiting for author Needs an action taken by the original poster label Oct 30, 2023
Copy link
Owner

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Great, thanks! Just one small nitpick on the opts variable name - I can merge in a few days if you don't have strong objection to renaming.

src/create/createRerunSuggestion.test.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Nov 8, 2023

👋 @GV14982 still interested in the opts rename? No worries if not, I just don't want to rudely take over 😄

@GV14982
Copy link
Contributor Author

GV14982 commented Nov 8, 2023

Oh sorry, I misunderstood your earlier comment. Yeah by all means!

@JoshuaKGoldberg JoshuaKGoldberg merged commit 5978879 into JoshuaKGoldberg:main Nov 9, 2023
16 checks passed
Copy link

github-actions bot commented Nov 9, 2023

🎉 This is included in version v1.43.1 🎉

The release is available on:

Cheers! 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug: Rerun suggestions don't factor in --base
2 participants