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

Create the build:dev npm script aliased to the current build script #2658

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

layoutd
Copy link
Collaborator

@layoutd layoutd commented Nov 2, 2023

Changes proposed in this Pull Request:

Adding the build:dev npm script for consistency with other projects. It's currently just aliased to the existing build script, although it seems like build in most repos actually ends by building the archive.

Detailed test instructions:

  1. See that both npm run build and npm run build:dev work and do the same thing (build assets and translations.

@layoutd layoutd added the changelog: none Skip changelog entry for this PR label Nov 2, 2023
@layoutd layoutd self-assigned this Nov 2, 2023
@github-actions github-actions bot added the changelog: dev Developer-facing only change. label Nov 2, 2023
@layoutd layoutd removed the changelog: dev Developer-facing only change. label Nov 2, 2023
@layoutd layoutd requested a review from a team November 3, 2023 10:59
Copy link
Member

@ecgan ecgan left a comment

Choose a reason for hiding this comment

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

Tested. They kinda work the same - they both build assets and translations.

However, npm run build:dev does not trigger prebuild (to run composer install) and postbuild (creating zip archive).

I am wondering: why do we want to add this build:dev script, since it is doing "the same thing"? Is there a pain point that you are experiencing from its inconsistency with other projects? 🤔

I looked into https://github.com/woocommerce/google-listings-and-ads/blob/e95a1151cc3ed9dcb902ead95e65c1cd3cc6ff1b/package.json#L93 and I think it has pretty good setup there (look for build and dev npm scripts). Maybe we can use that as a reference?

@layoutd
Copy link
Collaborator Author

layoutd commented Dec 14, 2023

I am wondering: why do we want to add this build:dev script, since it is doing "the same thing"? Is there a pain point that you are experiencing from its inconsistency with other projects? 🤔

My thought process is essentially to reduce brain cycles. We have a bit of a hodgepodge of commands: some repos use build:dev, some use dev, and this one currently uses build. My end goal is standardize the command to build all our extensions for easier testing when jumping across repos (think: compatibility testing, etc.).

Copy link
Member

@ecgan ecgan left a comment

Choose a reason for hiding this comment

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

Okay, let's do it! 👍 We can iterate on it along the way. 🙂

@layoutd layoutd merged commit 82aa6b5 into develop Dec 22, 2023
1 check passed
@layoutd layoutd deleted the dev/build-dev-npm-action branch December 22, 2023 14:07
@layoutd
Copy link
Collaborator Author

layoutd commented Dec 22, 2023

Thanks for the assistance and feedback, @ecgan 🙇🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: none Skip changelog entry for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants