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

Upgrade stylelint to 14.0.0 #66

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rsstdd
Copy link

@rsstdd rsstdd commented Nov 18, 2021

Updates stylelint and addresses issue #65.

@alies-dev
Copy link

hey @igorkamyshev @YozhikM Any news on this PR?

@igorkamyshev
Copy link
Contributor

hey @igorkamyshev @YozhikM Any news on this PR?

Hey. Unfortunately, I have no permissions to merge PRs in this repo.

@alexander-akait
Copy link
Collaborator

@rsstdd Ping me on this weekend if there is no answer

@benbender
Copy link

Ping @alexander-akait :)

@rsstdd
Copy link
Author

rsstdd commented Dec 12, 2021

Ping @alexander-akait!

@thibaudcolas
Copy link
Contributor

thibaudcolas commented Dec 13, 2021

It looks like this plugin could use additional maintenance love – do the people currently involved with it take applications for new maintainers? I’d happily volunteer some of my time, accessibility-oriented rules feel invaluable for the whole stylelint ecosystem.

Apart from this peerDeps compatibility change, the project should also migrate from Travis to GitHub Actions like other stylelint projects have.

@ModulesUnraveled
Copy link

Yes! I just tried to install this, and found it's not compatible with stylelint 14. This is sorely needed. Thank you!

@alexander-akait
Copy link
Collaborator

alexander-akait commented Feb 17, 2022

Sorry, I am very busy, maybe someone wants to send a PR? That is PR 😄

@alexander-akait
Copy link
Collaborator

Can we add github actions (i.e. CI) here to run tests, because it is hard what it works fine with new version without problems?

@ModulesUnraveled
Copy link

It appears that @YozhikM is the only one with access to do that sort of thing. And he hasn't been active in a couple of years? I'm going to work on a fork to bring things up to date, unless or until this one can be maintained.

@alexander-akait
Copy link
Collaborator

alexander-akait commented Feb 17, 2022

No need to do fork, we need just add CI file here and I will merge

@benbender
Copy link

@alexander-akait implementation of a CI-workflow shouldn't be part of this PR imho. Separate topic, separate PR.

@alexander-akait
Copy link
Collaborator

alexander-akait commented Feb 17, 2022

Sorry, I can't merge it without test, I understand what it should be part of another commit (but I don't understand why do not include it here), but I can't merge it without verification, it can be broken

@ModulesUnraveled
Copy link

I'm not sure how to add to a fork PR like this. But this should work if it's created at .github/workflows/test.yml

on:
  pull_request:
    branches: ['master']
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - name: Checkout
        uses: actions/checkout@v2
      - name: Install Node.js
        uses: actions/setup-node@v2
        with:
          node-version: 16.13
      - name: Install
        run: npm ci
      - run: npm run test

@ModulesUnraveled
Copy link

Then, ideally, you'd update the settings for this repo to protect the master branch and require tests to pass before merging. Something like this: (but with the master branch, instead of main in the screenshot)
Screen Shot 2022-02-17 at 7 04 06 AM

@benbender
Copy link

as we cant add to this pr, here is a separate one: #67

@benbender
Copy link

Additionally: The tests are clean for stylelint 14, but the (dev-)deps are vastly outdated and full security-issues and would need a serious overhault. This, in turn, would need a decision about the build-infrastructure and clean handling of cjs/esm as it is mixed atm and fails for upgraded deps.

@trdyer
Copy link

trdyer commented Apr 11, 2022

@alexander-akait Any chances on getting #67 merged so we can get movement on this?

@winston0410
Copy link

Hi will this PR be merged?

@thibaudcolas
Copy link
Contributor

For people following this discussion – there’s now talks of either adding some of those rules to Stylelint itself or creating another community plugin similar to stylelint-a11y here: stylelint/stylelint#6212.

@liyokuna
Copy link

Hello, any news about this PR ?

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

10 participants