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

fix/few packaging fixes #469

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

fix/few packaging fixes #469

wants to merge 15 commits into from

Conversation

jokesterfr
Copy link
Contributor

@jokesterfr jokesterfr commented Jan 15, 2025

Changes summary

  • Remove unecessary .gitkeep file
  • Git Ignore slight review
  • Lint the readme file
  • Makefile
    • Reintroduce git clean
    • Differentiate SEM_VERSION from VERSION
    • Enhance output of the help command
    • Remove .PHONY on top of config.php target

Questions

Makefile: should we keep using vendor-clean when we call make?

default: bundle
bundle: php-scoper ...
php-scoper: php-scoper-add-prefix ...
php-scoper-add-prefix: vendor-clean ...
vendor-clean:
	rm -rf ./vendor

Makefile: can we rename bundle to build?

Definition of "bundle": tie or roll up (a number of things) together as though into a parcel.

I see it as a confusing name, in particular with the "zip" artefact. In fact there is a BUNDLE section with BUNDLE_ZIP. I would rather prefer to speak about "build" when you gather all deps, and bundle when you roll up them altogether, as a zip archive.

Makefile the "config.php" file is essential to make the project work. However there is no creation of such file when calling make with a cleaned project. Furthermore this seems to be used in a complex docker-only process:

.PHONY: config.php
config.php:
	@docker exec -w ${CONTAINER_INSTALL_DIR} phpunit \
		sh -c "if [ ! -f ./config.php ]; then cp ./config.dist.php ./config.php; fi"

platform-module-install: config.php ...

I guess removing the .PHONY and letting build the config file from make is a better choice. Do we need a distinct configuration file than default for a docker test?

.zip-contents Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
@@ -19,16 +19,13 @@ composer-dev.json
composer-dev.lock
.env

### PHP Scoper ###
vendor-scoped/
Copy link
Contributor

Choose a reason for hiding this comment

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

That dir is not supposed to survive the scoping process

Copy link
Contributor Author

@jokesterfr jokesterfr Jan 16, 2025

Choose a reason for hiding this comment

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

It is "flashing" in my IDE file browser just the time the scoping starts. It's annoying to my old eyes :D

.zip-contents Outdated Show resolved Hide resolved
@hschoenenberger
Copy link
Contributor

Makefile the "config.php" file is essential to make the project work. However there is no creation of such file when calling make with a cleaned project. Furthermore this seems to be used in a complex docker-only process:

.PHONY: config.php
config.php:
	@docker exec -w ${CONTAINER_INSTALL_DIR} phpunit \
		sh -c "if [ ! -f ./config.php ]; then cp ./config.dist.php ./config.php; fi"

platform-module-install: config.php ...

I guess removing the .PHONY and letting build the config file from make is a better choice. Do we need a distinct configuration file than default for a docker test?

Just to be clear I need to be able to build locally for local, prod, preprod etc. Maybe we can add a test config file then build explicitly with it at CI time ?

@jokesterfr
Copy link
Contributor Author

jokesterfr commented Jan 16, 2025

Just to be clear I need to be able to build locally for local, prod, preprod etc. Maybe we can add a test config file then build explicitly with it at CI time ?

My point is prior to any CI consideration there should be an easy way to locally:

  • build the project without any zip, ex: make
  • bundle an e2e/local zip, ex: make zip-e2e
  • bundle an integration zip, ex: make zip-inte
  • bundle a pre-production zip, ex: make zip-preprod
  • bundle a production zip, ex: make zip-prod

And a handy all-in-one command, ex: make zip, calling all the upper-mentioned commands.
Output zips will be stored in the gitignored ./dist directory.

Step 1 is building and step 2 is bundling, and the last one is optional.

@jokesterfr jokesterfr force-pushed the fix/few-packaging-fixes branch from c6ee081 to 37f804b Compare January 16, 2025 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants