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

Updating all-contributors-cli dependency #2360

Closed
wants to merge 4 commits into from
Closed

Updating all-contributors-cli dependency #2360

wants to merge 4 commits into from

Conversation

jdalrymple
Copy link

What Changed

Updated the all-contributors-cli dependency

Why

To support new configuration within the cli

Todo:

  • Add tests
  • Add docs

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

@hipstersmoothie
Copy link
Collaborator

@jdalrymple doesn't look like the lock file is fully updated

@jdalrymple
Copy link
Author

Oh darn! Will do. Which version of yarn? I want to make sure when i do the update it doesnt refresh all of the yarn files

@hipstersmoothie
Copy link
Collaborator

we're on v1

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #2360 (a4be071) into main (b9aa3db) will increase coverage by 2.25%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2360      +/-   ##
==========================================
+ Coverage   80.55%   82.81%   +2.25%     
==========================================
  Files          69       69              
  Lines        5668     5614      -54     
  Branches     1330     1317      -13     
==========================================
+ Hits         4566     4649      +83     
+ Misses        719      711       -8     
+ Partials      383      254     -129     
Impacted Files Coverage Δ
packages/core/src/auto.ts 80.94% <ø> (+3.01%) ⬆️
plugins/exec/src/index.ts 95.40% <100.00%> (+2.22%) ⬆️

... and 35 files with indirect coverage changes

@jdalrymple
Copy link
Author

Looking at the failed tasks, should i be addressing the typing errors in other files?

@hipstersmoothie
Copy link
Collaborator

Looks like there is only lint failures now. Once that's good I can merge

@jdalrymple
Copy link
Author

Looks like there is only lint failures now. Once that's good I can merge

Yes, but in a file that wasnt changed...?

/home/runner/work/auto/auto/packages/core/src/release.ts
Error: 1:33 error Unable to resolve path to module 'type-fest' import/no-unresolved

@jdalrymple
Copy link
Author

Looking into it! Might be related to yarn

@jdalrymple
Copy link
Author

Scratch that, i have no idea. When i run the lint script locally it also receive many more errors that have absolutely nothing to do with the code. For example:

/home/flynn/Projects/personal/auto/packages/cli/src/run.ts
7:3 error 'IInfoOptions' is defined but never used @typescript-eslint/no-unused-vars

@jdalrymple
Copy link
Author

Maybe the update of the yarn lock is using a more recent version of the eslint lib since the pkg.json doesnt lock them to a specific version?

@hipstersmoothie
Copy link
Collaborator

I'll pull this today and figure it out!

@jdalrymple
Copy link
Author

Adding the type prefix seemed to fix most of this issues, but its quite a bit of changes: https://stackoverflow.com/a/71375585/1812936

@jdalrymple
Copy link
Author

Using this lib: https://github.com/import-js/eslint-import-resolver-typescript, might help, though my tests with it didnt solve the issue

@jdalrymple jdalrymple closed this by deleting the head repository Dec 26, 2023
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.

None yet

2 participants