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

feat(type-safe-api): add commitGeneratedCode option to control generated code #814

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

jstrunk
Copy link
Contributor

@jstrunk jstrunk commented Jul 19, 2024

This commit introduces a new commitGeneratedCode option to the TypeSafeApiProject, which allows controlling whether generated code should be committed or ignored for all generated projects.

The main changes include:

  • Add a commitGeneratedCode option to the TypeSafeApiProject and related options interfaces.
  • Set the default value of commitGeneratedCode to false.
  • Conditionally add patterns to .gitignore based on the commitGeneratedCode option for all generated projects.
  • Update tests to cover the new commitGeneratedCode option.

By default, the generated code will be ignored in the repository.

Copy link

nx-cloud bot commented Jul 25, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5a93d6a. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@agdimech
Copy link
Contributor

Hi @jstrunk,

Getting some test failures - can you please update?

@jstrunk
Copy link
Contributor Author

jstrunk commented Jul 26, 2024

Hi @agdimech,

Thanks for pointing that out. The other snapshots got updated for the Python projects now that those are committed by default.

@agdimech
Copy link
Contributor

agdimech commented Aug 1, 2024

I've been chatting with @cogwirrel about this and they mentioned that having a different default for Python doesn't seem right which I tend to agree with. A possible alternative solution could be:

1.) Write a temporary blank readme during install (just like we do for init.py here)
2.) Unset the readme option that gets written to the pyproject.toml file by projen - my guess is that newer versions of poetry are validating that the file that specifies exists as part of install

@jstrunk
Copy link
Contributor Author

jstrunk commented Aug 1, 2024

I've been chatting with @cogwirrel about this and they mentioned that having a different default for Python doesn't seem right which I tend to agree with. A possible alternative solution could be:

1.) Write a temporary blank readme during install (just like we do for init.py here) 2.) Unset the readme option that gets written to the pyproject.toml file by projen - my guess is that newer versions of poetry are validating that the file that specifies exists as part of install

I agree. I have validated this idea. I'll make a new PR with solution 1. Then I'll clean up this PR.

…ted code

This commit introduces a new `commitGeneratedCode` option to the
TypeSafeApiProject, which allows controlling whether generated code should be
committed or ignored for all generated projects.

The main changes include:

- Add a `commitGeneratedCode` option to the TypeSafeApiProject and related
options interfaces.
- Set the default value of `commitGeneratedCode` to `false`.
- Conditionally add patterns to .gitignore based on the `commitGeneratedCode`
option for all generated projects.
- Update tests to cover the new `commitGeneratedCode` option.

By default, the generated code will be ignored in the repository, except for
Python projects where it will be included to allow for easier distribution and
deployment of the generated artifacts using Poetry.

Fixes: aws#813
@jstrunk
Copy link
Contributor Author

jstrunk commented Aug 1, 2024

@agdimech I've updated this PR to remove the different defaults for Python. I created #818 to fix #813.

@jstrunk
Copy link
Contributor Author

jstrunk commented Aug 1, 2024

This PR partially addresses #819

@agdimech agdimech merged commit cf73714 into aws:mainline Aug 6, 2024
6 checks passed
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