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

[Feature] Added features to the premake cli wrapper #17398

Open
wants to merge 1 commit into
base: develop2
Choose a base branch
from

Conversation

Ohjurot
Copy link
Contributor

@Ohjurot Ohjurot commented Dec 1, 2024

Changelog: Feature:
Addes features to the preamke5 cli wrapper:

  • premake5 action (vsXXXX or gmake2) gets auto-detected in constructor but can be changed via public member.
  • The path and filename to the root premake5.lua file is changeable
  • Custom premake5 command line arguments can be specified via public dict arguments

Thees changes are required in order to fully support calling the premake5 executable. This lays the foundation for a PremakeToolchain.

This is one of the key components of #14724 but in a modified version. I plan to add the other main feature (build) as a follow up under PremakeToolchain. As I have understand as long as premake does not have a build feature. Which it does not have. It should be "chained" as a Toolchain that would utilize the cli wrapper and the platform specific build system wrapper (MSBuild or a direct make call) other than being integrated in the cli wrapper. Correct?

Would we need to add any test coverage other than the "basic" test @memsharded added? Like checking that arguments work etc.)

Currently there is (as previously discussed) no documentation for the whole premake feature set. I assume we will stay like this for now. I would however recommend to document the feature as soon as premake is feature complete.

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

- Modifiable premake action (autodetected in constructor)
- Modifiable lua file location
- Custom user actions (defined within the premake5.lua file) exposed as 'arguments'
@Ohjurot Ohjurot mentioned this pull request Dec 1, 2024
5 tasks
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

It would be great to add a couple of unit test illustrating the usage and functionality of these changes. I understand they cover some customization cases, which seems legit, but seeing the actual usage tests help to polish the UI and also to get them covered for the future to avoid possible future breakages.

Thanks for your contribution @Ohjurot !

@Ohjurot
Copy link
Contributor Author

Ohjurot commented Dec 19, 2024

Thanks for the feedback @memsharded! I will implement as soon as I have time.

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.

2 participants