Skip to content

Conversation

@bblommers
Copy link
Contributor

This should make it a little easier to release this library.

If we go ahead with this, we will probably need to configure PyPi and add this library to the list of Trusted Publishers.

FYI @alexrashed as our CI expert 🙂

@bblommers bblommers requested a review from alexrashed November 28, 2025 11:18
Makefile Outdated
touch $(VENV_DIR)/bin/activate

install: venv
install: $(VENV_ACTIVATE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running make install on my machine didn't actually do anything - it just said:

make: Nothing to be done for 'install'.

I don't know why, or whether this is a system-specific thing? But it does work using the $(VENV_ACTIVATE) command, both locally and in the CI

Copy link
Member

Choose a reason for hiding this comment

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

For what I can see, the make target dependencies are: install -> venv -> .venv/bin/activate -> pyproject.toml
We should add all non-file-targets to the .PHONY, but I don't think that would make a difference.
I think in your case it's really just that the change date of .venv/bin/activate is later than pyproject.toml? In this case make would detect that .venv/bin/activate is newer than the pyproject.toml (so it does not have to be rebuilt).
This in turn means that the venv and the install targets do not have to be executed.
But that actually might be correct in this case?

If you want to make sure that the install target is always executed, then I would recommend to move the editable pip install command to the install target and add the install target to .PHONY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should have specified actually that make install didn't do anything - despite the fact that there was no .venv on my system. Running make install lint would then fail, because the linter could not be executed without a virtual environment.

However, I cannot reproduce this anymore. Everything now suddenly works as it should, so I've reverted this change.

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Nice! This is looking great! 🤩
Thanks for streamlining the release of the package here. I think this should reduce the turnover time for the other PRs you might have out there, and help get changes out! 💯

Comment on lines +15 to +17
environment:
name: pypi
url: https://pypi.org/p/localstack-snapshot
Copy link
Member

Choose a reason for hiding this comment

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

praise: Nice! Following best practices here with the env! I added this env + workflow + repo as trusted publisher to the project on PyPi!

@bblommers bblommers force-pushed the admin-release-workflow branch from 42544c5 to cab9e53 Compare December 2, 2025 12:42
@bblommers bblommers merged commit fb24ac6 into main Dec 2, 2025
3 checks passed
@bblommers
Copy link
Contributor Author

bblommers commented Dec 2, 2025

Thanks for helping out @alexrashed, despite being OOO! I just managed to release 0.3.1, so it looks like everything works OK 👍

@alexrashed alexrashed deleted the admin-release-workflow branch December 2, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants