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

chore: bump detective-postcss to v6 #319

Merged
merged 1 commit into from Jan 28, 2023

Conversation

PabloLION
Copy link
Collaborator

Why

When installing madge with yarn and node v18.2.0. This error was thrown

error detective-postcss@5.1.1: The engine "node" is incompatible with this module. Expected version "12.x || 14.x || 16.x". Got "18.2.0"

How

I tried yarn install in a fork of this repo and it had the same error.
After yarn update detective-postcss@latest, there's no more error on yarn install and yarn test still passes.
Please consider merge this PR (and better, add some dependency checker in this code base).

TIA

@fdc-viktor-luft
Copy link
Collaborator

This update is required for Node 18 support

@fdc-viktor-luft
Copy link
Collaborator

Why don't get approved PRs not get merged here? @kkamik or @PabloLION can't you merge this and release a new version? Also other good PRs are pending a long time. What is the reason?

@PabloLION
Copy link
Collaborator Author

Hi @fdc-viktor-luft . This repo is still under maintenance (by @pahen, and I'm not authorized to merge) since new PRs like #325 are still getting merged.
Personally I don't think creating a fork for only one PR is a very good idea. Maybe we can ask @pahen if there's anything wrong? I didn't test if this breaks node 14 support so more works needs to be done.

To me the fork is needed only when maintainers have very different (and absurd to me) expectations of this repo like "We only want to keep this available for node 11". If so, I'll publish a new NPM package.

@PabloLION
Copy link
Collaborator Author

and adopt this project. I think this tool helped a lot!

@pahen
Copy link
Owner

pahen commented Nov 11, 2022

Why don't get approved PRs not get merged here? @kkamik or @PabloLION can't you merge this and release a new version? Also other good PRs are pending a long time. What is the reason?

I’m on paternity leave until January 2023 and building a new house so there’s no time to maintain this project right now 😢 I’ll see if I can get help maintaining this project. Any suggestions?

@fdc-viktor-luft
Copy link
Collaborator

fdc-viktor-luft commented Nov 11, 2022

@pahen That can happen. Don't worry. We just need someone else who can take care instead of you. Maybe you share write access to someone who volunteers.

I will be on parental leave soon, too. Will be back on February 2023, but I can offer to take care of smaller tasks. I'm maintaining and own also several npm packages, but none so popular for sure.

@PabloLION wants to volunteer maybe too? 😅

@PabloLION
Copy link
Collaborator Author

@pahen @fdc-viktor-luft yea sure. Just to be clear I have no experience maintaining any popular package like this one. I'm currently between jobs. I'll try my best until I become busy again, but I'm sure that'll be after January 2023.

@PabloLION
Copy link
Collaborator Author

I've posted a new issue #334 to discuss about the new topic in order to keep the discussion here focused on this PR.

@StanislavKharchenko
Copy link

Any updates?
This stop to switch to Node 18

@fdc-viktor-luft
Copy link
Collaborator

Sadly not. If you're using e.g. pnpm you can use this in your package.json as current workaround in order to use Node 18:

{
  "pnpm": {
    "overrides": {
      "detective-postcss": "6.1.0"
    }
  }
}

@pahen pahen self-requested a review January 27, 2023 14:52
Copy link
Collaborator

@kamiazya kamiazya left a comment

Choose a reason for hiding this comment

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

LGTM!

@kamiazya kamiazya added this to the 6.0.0 milestone Jan 28, 2023
@kamiazya kamiazya merged commit 372668a into pahen:master Jan 28, 2023
@kamiazya kamiazya mentioned this pull request Jan 29, 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

6 participants