Skip to content

package lock dependencies #4

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

Closed
GFdevelop opened this issue Aug 20, 2020 · 7 comments · Fixed by #10
Closed

package lock dependencies #4

GFdevelop opened this issue Aug 20, 2020 · 7 comments · Fixed by #10
Labels
dependencies Pull requests that update a dependency file help wanted Extra attention is needed

Comments

@GFdevelop
Copy link

PeerDependencies warnings:

"@commitlint/cli": "^8.3.5",

don't match:
"@commitlint/cli": "^9.1.1",

@commitlint/cli must be v9.1.2 instead of v9.1.1
commitizen must be v4.1.2 instad of v4.0.3
there is also eslint v7.7.0

@martinmcwhorter martinmcwhorter added the dependencies Pull requests that update a dependency file label Aug 24, 2020
@martinmcwhorter
Copy link
Owner

The short version is -

  • Commitiquette was developed using @types from definitelytyped
  • Some newer deps now provide there own types

So updating the deps are not always as simple as they would otherwise be.

I took a stab at updating all the conflicting types to the package provided types remove the definetlytyped (@types) ones. It quickly grew in complexity.

I would like to take a phased approach:

  1. update the dependancies, use the existing definetlytyped types
  2. coerce leaf types to use provided typed
  3. update branches to use types
  4. remove the overlapping definitelytyped types completely.

I have not spent the time investigating how to achieve number 1 yet.

@xeptore
Copy link
Contributor

xeptore commented Mar 30, 2021

@martinmcwhorter, I tried updating all dependencies to their latest versions, it took around 2 hours to only find out what and how the types has changed, specially commitlint types. But, as you said, it quickly became complex and even impossible to convert in some modules. For example, the validator function in utils module does a lot of validation which I think is not necessary if we assume the configuration the engine receives is correct, and move all the debugging to end-user in case of an error caused by wrong configuration.
After these, what do you think about a refactor in some modules, maybe in the structure of the codebase in whole?

@martinmcwhorter
Copy link
Owner

I agree.

I have would like to migrate to the supplies type declarations, but have no interest in using any of the runtime validation. At least for now.

What did you have in mind for the refactor?

@xeptore
Copy link
Contributor

xeptore commented Mar 31, 2021

I think we can eliminate many internal validations of configuration object we receive and rule-existence-checks by having a defaults-applier layer before the engine which applies default values to user configuration, if commitlint itself doesn't do this already.

BTW, I will try to only upgrade dependencies to their latest version without introducing too much change in working mechanisms in the couple of days.

@martinmcwhorter
Copy link
Owner

@xeptore The changes look good, do you want to make a PR into my repo?

@xeptore
Copy link
Contributor

xeptore commented Apr 1, 2021

@martinmcwhorter, no difference. I'm OK with either ways. What do you prefer?

@martinmcwhorter
Copy link
Owner

Not sure I understand what you are asking.

The PR you linked to is a PR from one branch in your fork to another branch, all in your fork.

For me to merge the changes into my repo -- please make a PR back into this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants