atk4 / ui

Robust and easy to use PHP Framework for Web Apps
https://atk4-ui.readthedocs.io
MIT License
440 stars 104 forks source link

Fix typos #2077

Closed szepeviktor closed 1 year ago

szepeviktor commented 1 year ago

From https://github.com/atk4/data/issues/1104

@mvorisek

szepeviktor commented 1 year ago

Smoke (latest, CodingStyle) started failing 2 days ago. -> #2078

szepeviktor commented 1 year ago

May I contribute a new CI workflow that regenerates (and commits) generated files?

mvorisek commented 1 year ago

May I contribute a new CI workflow that regenerates (and commits) generated files?

In past we have been using that, but it turned out more commits are harded to work with, so we have CI to assert all files are up-to-date, but you have to commit them manually from your side - you changed JS files, you need to rebuild them and commit.

szepeviktor commented 1 year ago

you changed JS files, you need to rebuild them and commit

Could you please do that for me this time?

mvorisek commented 1 year ago

In js/ run npm install and npm build.

szepeviktor commented 1 year ago

Done.

szepeviktor commented 1 year ago

It would be much better (no stomach pain for viktor) to set engines in package.json.

{
    "engines": {
        "node": "^18.12",
        "npm": "^9.2.0",
        "yarn": "please-use-npm"
    }
}

And npm install should be npm ci to leave the lock file unchanged.

mvorisek commented 1 year ago

It would be much better (no stomach pain for viktor) to set engines in package.json.

{
    "engines": {
        "node": "^18.12",
        "npm": "^9.2.0",
        "yarn": "please-use-npm"
    }
}

And npm install should be npm ci to leave the lock file unchanged.

we use npm ci in CI, with npm install, did the lock file change?

node / npm - why?

yarn - we should be compatible with yarn

szepeviktor commented 1 year ago

we use npm ci in CI

Here "ci" does not mean "continuous integration" but clean install. The goal is to have generated files updated without package versions change.

szepeviktor commented 1 year ago

with npm install, did the lock file change?

I was running only npm ci.

mvorisek commented 1 year ago

How viable is to add https://github.com/crate-ci/typos into our CI for atk4/{core, data, ui}? Can we run it without any false positives out of the box?

szepeviktor commented 1 year ago

yarn - we should be compatible with yarn

Here is how to do that.

yarn import # import npm's package-lock.json
yarn install --frozen-lockfile
szepeviktor commented 1 year ago

How viable is to add https://github.com/crate-ci/typos into our CI

That is a default workflow in my work. https://github.com/szepeviktor/debian-server-tools/blob/master/webserver/Continuous-integration-Continuous-delivery-spelling.yml

I was planning to improve your CI workflows. for example https://github.com/szepeviktor/debian-server-tools/blob/master/webserver/Continuous-integration-Continuous-delivery.yml