brunob / leaflet.fullscreen

Leaflet.Control.FullScreen for Leaflet
https://brunob.github.io/leaflet.fullscreen/
MIT License
376 stars 107 forks source link

Rework of the lintering and formatting process #119

Closed KristjanESPERANTO closed 7 months ago

KristjanESPERANTO commented 8 months ago

A second attempt to rebuild the linter and formatter process. I canceled the first one because it did not contain the new flat configuration of ESLint. See also the discussion in PR #115.

Changes

There should be no functional changes.

BePo65 commented 8 months ago

This looks very good to me.

brunob commented 7 months ago

Done, thx again @KristjanESPERANTO & @BePo65 :)

Now we may try to add a CI on the repo in order to run lint on every PR, what do yout think ?

KristjanESPERANTO commented 7 months ago

Now we may try to add a CI on the repo in order to run lint on every PR, what do yout think ?

I also think that this would be good step. I don't have an overview of what is currently best practice here. I usually use husky, which already prevents me from committing unlinted code. But an action like this one doesn't seem so wrong to me either.

brunob commented 7 months ago

But an action like this one doesn't seem so wrong to me either.

This looks good https://github.com/marketplace/actions/lint-action#javascript-example-eslint-and-prettier and seems almost identical to what we do in https://github.com/leaflet-extras/leaflet-providers/blob/master/.github/workflows/ci.yml

brunob commented 7 months ago

Proposition here https://github.com/brunob/leaflet.fullscreen/pull/120

BePo65 commented 7 months ago

On my repos I use github actions (e.g. one of my repos) - that is what you do in pr #120.
Why do you add npm ci? We don't run or build any code.

And you can make this workflow mandatory for pul requests (see github documentation).

brunob commented 7 months ago

On my repos I use github actions (e.g. one of my repos) - that is what you do in pr #120. Why do you add npm ci? We don't run or build any code.

And you can make this workflow mandatory for pul requests (see github documentation).

I've tried without npm ci and it fails https://github.com/brunob/leaflet.fullscreen/actions/runs/7614739500/job/20737709233?pr=120 FTR it worked with npm ci https://github.com/brunob/leaflet.fullscreen/actions/runs/7614569335

BePo65 commented 7 months ago

Sorry, I was wrong. but of course we need npm ci otherwise prettier is not installed :-(

KristjanESPERANTO commented 7 months ago

Proposition here https://github.com/brunob/leaflet.fullscreen/pull/120

Good solution :+1:

brunob commented 7 months ago

done, i've also publisher a patch release

i think now we can propose to transfer the ownership of this repo to @Leaflet org :)

KristjanESPERANTO commented 7 months ago

Whereby in the past I often had the feeling that plugins in the @Leaflet repo are very neglected...

brunob commented 7 months ago

If this plan is not successful, we still have plan B by creating a team of maintainers here :)

brunob commented 2 months ago

@KristjanESPERANTO i see that we are still using eslint v8, what do you think about adding something like https://github.com/leaflet-extras/leaflet-providers/blob/master/.github/dependabot.yml in order too keep our dependencies up to date ?

KristjanESPERANTO commented 2 months ago

@KristjanESPERANTO i see that we are still using eslint v8, what do you think about adding something like https://github.com/leaflet-extras/leaflet-providers/blob/master/.github/dependabot.yml in order too keep our dependencies up to date ?

Good idea. I've just prepared a PR: #126.