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: save exclusion rules when bumping the CFN spec #22788

Merged
merged 7 commits into from Dec 7, 2022

Conversation

otaviomacedo
Copy link
Contributor

@otaviomacedo otaviomacedo commented Nov 4, 2022

bump-cfnspec.sh now does a full build, with the env variable AWSLINT_SAVE=true. This has the effect of adding awslint exclusion rules to any package that violates awslint rules after it is created (e.g., new attributes in CFN that don't yet exist in the corresponding L2). As part of the script, these changes will be commited to Git.

Tested locally on a clean build (except for a change in the package.json of a construct module to check if the changes were reverted after the build) and on the CodeBuild job, by pointing it to this branch.


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Nov 4, 2022

@aws-cdk-automation aws-cdk-automation requested a review from a team November 4, 2022 16:58
@github-actions github-actions bot added the p2 label Nov 4, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 4, 2022
rix0rrr
rix0rrr previously requested changes Nov 4, 2022
Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

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

You need to run the full build AFTER the spec update, not before.

Did you test this on the actual CodeBuild job?

@otaviomacedo otaviomacedo changed the title chore: save exlusion rules when bumping the CFN spec chore: save exclusion rules when bumping the CFN spec Nov 7, 2022
@otaviomacedo otaviomacedo reopened this Nov 29, 2022
@otaviomacedo otaviomacedo dismissed rix0rrr’s stale review November 29, 2022 11:01

Changed to running the full build after the spec update.

@mrgrain
Copy link
Contributor

mrgrain commented Nov 29, 2022

I think we should do this for v1.

Still undecided on v2, but leaning towards the linter is actually useful for v2 to enforce good coverage.

@rix0rrr @otaviomacedo WDYT?

@mergify
Copy link
Contributor

mergify bot commented Dec 7, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: a4f8906
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 4e7fdae into main Dec 7, 2022
@mergify mergify bot deleted the otaviom/bump-cfnspec-awslint-save branch December 7, 2022 16:49
@mergify
Copy link
Contributor

mergify bot commented Dec 7, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

RomainMuller added a commit that referenced this pull request Dec 8, 2022
This reverts commit 4e7fdae, because
the problem it was intended to solve was fixed in a different way, and
this particular approach causes automation to time-out after >1H.
mergify bot pushed a commit that referenced this pull request Dec 8, 2022
…" (#23282)

This reverts commit 4e7fdae, because the problem it was intended to solve was fixed in a different way, and this particular approach causes automation to time-out after >1H.


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Dec 9, 2022
`bump-cfnspec.sh` now does a full build, with the env variable `AWSLINT_SAVE=true`. This has the effect of adding awslint exclusion rules to any package that violates awslint rules after it is created (e.g., new attributes in CFN that don't yet exist in the corresponding L2). As part of the script, these changes will be commited to Git.

Tested locally on a clean build (except for a change in the `package.json` of a construct module to check if the changes were reverted after the build) and on the CodeBuild job, by pointing it to this branch.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Dec 9, 2022
…2788)" (aws#23282)

This reverts commit 4e7fdae, because the problem it was intended to solve was fixed in a different way, and this particular approach causes automation to time-out after >1H.


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Jan 20, 2023
`bump-cfnspec.sh` now does a full build, with the env variable `AWSLINT_SAVE=true`. This has the effect of adding awslint exclusion rules to any package that violates awslint rules after it is created (e.g., new attributes in CFN that don't yet exist in the corresponding L2). As part of the script, these changes will be commited to Git.

Tested locally on a clean build (except for a change in the `package.json` of a construct module to check if the changes were reverted after the build) and on the CodeBuild job, by pointing it to this branch.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Jan 20, 2023
…2788)" (aws#23282)

This reverts commit 4e7fdae, because the problem it was intended to solve was fixed in a different way, and this particular approach causes automation to time-out after >1H.


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Feb 22, 2023
`bump-cfnspec.sh` now does a full build, with the env variable `AWSLINT_SAVE=true`. This has the effect of adding awslint exclusion rules to any package that violates awslint rules after it is created (e.g., new attributes in CFN that don't yet exist in the corresponding L2). As part of the script, these changes will be commited to Git.

Tested locally on a clean build (except for a change in the `package.json` of a construct module to check if the changes were reverted after the build) and on the CodeBuild job, by pointing it to this branch.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Feb 22, 2023
…2788)" (aws#23282)

This reverts commit 4e7fdae, because the problem it was intended to solve was fixed in a different way, and this particular approach causes automation to time-out after >1H.


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants