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.

Sign up for GitHub

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

Jump to bottom

fix(cli): cdk context --reset <number> does not work #10619

Merged
merged 2 commits into from
Oct 5, 2020
Merged

fix(cli): cdk context --reset <number> does not work #10619

merged 2 commits into from
Oct 5, 2020

Conversation

diesal11
Copy link
Contributor

@diesal11 diesal11 commented Oct 1, 2020 •
edited
Loading

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

Fixes #3033

@diesal11 diesal11 changed the title fix(aws-cdk): 'cdk context --reset <number>' does not work, fixes #3033 fix(aws-cdk): 'cdk context --reset <number>' does not work Oct 1, 2020
rix0rrr
rix0rrr approved these changes Oct 5, 2020
View reviewed changes
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.

Thanks!

@rix0rrr rix0rrr changed the title fix(aws-cdk): 'cdk context --reset <number>' does not work fix(cli): cdk context --reset <number> does not work Oct 5, 2020
@mergify
Copy link
Contributor

mergify bot commented Oct 5, 2020

Thank you for contributing! Your pull request will be updated from master 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: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 8c6cff1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Oct 5, 2020

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

@mergify mergify bot merged commit 3f62752 into aws:master Oct 5, 2020
@NetaNir NetaNir mentioned this pull request Oct 7, 2020
Merged
mergify bot pushed a commit that referenced this pull request Oct 7, 2020
@NetaNir
chore(cli): "cdk context" is broken (#10751)
4864fa0 
This [commit](#10619), while fixed the problem mentioned, broke the simplest functionally of:
```
cdk context
```
Now throws:

```
Cannot read property 'sort' of undefined
```
This PR reverts that change, and adds a simple unit test that will allow detecting this kind of breakage in unit tests.

We still need to fix the issue in the PR, opening the orginal [issue](#3033). 


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@NetaNir
Copy link
Contributor

NetaNir commented Oct 7, 2020

Hi @diesal11!
I had to revert this as it was breaking cdk context, I have added a simple unit test that should help with fixing this PR.
Let me know if you want to pick it up, of not I will. Thanks!

@diesal11
Copy link
Contributor Author

diesal11 commented Oct 7, 2020

No worries @NetaNir. I was concerned that the test coverage was a little low to be sure something like this wouldn't happen. My apologies as i should have mentioned that in the initial PR.

Jumping back on this now 👍

diesal11 pushed a commit to diesal11/aws-cdk that referenced this pull request Oct 7, 2020
@NetaNir @diesal11
chore(cli): "cdk context" is broken (aws#10751)
f0efab4 
This [commit](aws#10619), while fixed the problem mentioned, broke the simplest functionally of:
```
cdk context
```
Now throws:

```
Cannot read property 'sort' of undefined
```
This PR reverts that change, and adds a simple unit test that will allow detecting this kind of breakage in unit tests.

We still need to fix the issue in the PR, opening the orginal [issue](aws#3033).

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@diesal11 diesal11 mentioned this pull request Oct 7, 2020
Merged
@diesal11
Copy link
Contributor Author

diesal11 commented Oct 7, 2020

@NetaNir Fixed :)

I've had to create a new PR (#10753) as i had already deleted my fork which this PR was based off.

mergify bot pushed a commit that referenced this pull request Oct 19, 2020
@diesal11
fix(cli): cdk context --reset <number> does not work (#10753)
2f3a167 
This PR fixes #3033 while also addressing the issues that arose from #10619

----

*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
Reviewers

@rix0rrr rix0rrr rix0rrr approved these changes

Assignees

@rix0rrr rix0rrr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cdk context --reset <number> does not work
4 participants
@diesal11 @aws-cdk-automation @NetaNir @rix0rrr