wp-media / rocket-scripts

MIT License
0 stars 0 forks source link

Enhancement/automated pr wprocket and remove dist #12

Closed MathieuLamiot closed 2 months ago

MathieuLamiot commented 2 months ago

Description

Over the past 2 weeks, the use of this repository has been complicated as it is not clear for everyone how to use it to update the script. This PR fixes a few practices that could help: the dist files are not stored anymore, but build when needed ; when releasing a new version, a PR is opened on WP Rocket directly to manage the transition to wp-rocket repo.

The new GH action needs tokens and a branch on WP Rocket.

Documentation

User documentation

Technical documentation

NA

Type of change

Delete options that are not relevant.

New dependencies

NA

Risks

The new GH action will be need to be tested.

Checklists

Feature validation

Documentation

Code style

Observability

Risks

wordpressfan commented 2 months ago

@MathieuLamiot

Good point, I just want to share a small thing with you. This will work perfectly with release but during development phase we were using the rocket-scripts branch name in github instead of the release version in that case when dist directory is not there, we will not be able to copy those js files into WP Rocket assets. what do u think?

MathieuLamiot commented 2 months ago

Thanks for the feedback @wordpressfan I am not sure to understand, sorry 😬 In both actions, there is a npm run build to generate the dist/ files before publishing or sending to WP Rocket so that should be covered. If you are talking about manually importing from rocket-scripts into a WP Rocket version to test during development phase, then yes you are right. The dev would have to run npm run build in rocket-scripts first to get access to the files. Is that an issue?

wordpressfan commented 2 months ago

In both actions, there is a npm run build to generate the dist/ files before publishing or sending to WP Rocket so that should be covered.

Yes this would work with the release workflow and that's great.

If you are talking about manually importing from rocket-scripts into a WP Rocket version to test during development phase, then yes you are right. The dev would have to run npm run build in rocket-scripts first to get access to the files. Is that an issue?

How would they do that? u mean opening the branch they want in rocket-scripts and running build script then copying those files manually to WPR? When using the branch name inside package.json in WPR, we get the files from git and now we don't have dist in git so we have to move them manually, that's the case that I'm talking about.

MathieuLamiot commented 2 months ago

Ah right, got it now. Hmm let me think :D

MathieuLamiot commented 2 months ago

We could adjust wp-rocket to build wp-rocket-scripts intead of just copying the dist files?

We could add a npm script in package.json: "build:js:beacon": "cd node_modules/wp-rocket-scripts && npm install && npm run build && cd ../.. && gulp copy:js:beacon",

So that, after npm install, instead of doing npm run gulp build:js:beacon, we do npm run build:js:beacon. The script will build the dependency and copy the files. WDYT?

Seems to be working on my end

wordpressfan commented 2 months ago

I was thinking of something like that but with postinstall hook https://docs.npmjs.com/cli/v10/using-npm/scripts#npm-install

So once we run npm install we can check if the dist directory is not there so go to that directory and run the build command there so the files will be ready to be used by our WPR gulp task anytime, otherwise do nothing as dist is there already.