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

chore: dynamically generate GENERATED_SERVICES var #1008

Merged
merged 1 commit into from
Feb 2, 2022
Merged

chore: dynamically generate GENERATED_SERVICES var #1008

merged 1 commit into from
Feb 2, 2022

Conversation

dwerder
Copy link
Contributor

@dwerder dwerder commented Dec 15, 2021

Signed-off-by: Daniel Werdermann [email protected]

Description of your changes

The variable in the Makefile is now generated from the paths of the generatec-config.yaml files. So no more editingof the Makefile and no merge conflicts.

Fixes #861
Fixes #862

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

make reviewable works without changes.

@haarchri haarchri requested a review from muvaf December 15, 2021 11:47
@haarchri
Copy link
Member

good to go with excluded services - since we checked:
#1003
#1004
#1006

@dwerder
Copy link
Contributor Author

dwerder commented Dec 15, 2021

The force push commit added a more secure regex for the excludes.

@haarchri
Copy link
Member

as of review from @muvaf for #862 it would be great to see his review here

Makefile Outdated Show resolved Hide resolved
CODE_GENERATION.md Outdated Show resolved Hide resolved
Copy link
Member

@muvaf muvaf left a comment

Choose a reason for hiding this comment

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

Thanks @dwerder ! LGTM with only one thing to address. Great work!

Comment on lines 65 to 72
If `apis/<serviceid>` is created from scratch, please add the new service name to the list called GENERATED_SERVICES in [Makefile](https://github.com/crossplane/provider-aws/blob/master/Makefile#L10).

### Makefile

```bash
- GENERATED_SERVICES="apigatewayv2"
+ GENERATED_SERVICES="apigatewayv2,<serviceid>"
```
The serviceids are fetch from the paths of the generator-config.yaml automatically.

If this step fails for some reason, please raise an issue in [code-generator](https://github.com/aws-controllers-k8s/code-generator)
and mention that you're using Crossplane pipeline.
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the whole section as they don't need to know about this at all thanks to your discovery implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@muvaf muvaf merged commit 434c461 into crossplane-contrib:master Feb 2, 2022
tektondeploy pushed a commit to gtn3010/provider-aws that referenced this pull request Mar 12, 2024
…eplication-configuration-key-ref

feat(ref): add kms ref/selector in replication configuration
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.

Split Makefile GENERATED_SERVICES into one line per service to reduce the number of merge conflicts
3 participants