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: add ExternalCredentials metadata and v56 api version #390

Merged
merged 3 commits into from
Nov 12, 2022

Conversation

nodex0
Copy link
Contributor

@nodex0 nodex0 commented Nov 11, 2022

What does this pull request contain?

This Pull Request contains an additional metadata definition for the ExternalCredentials and the additional v56 API version metadata.


  • Added (for new features).
  • Changed (for changes in existing functionality).
  • Deprecated (for soon-to-be removed features).
  • Removed (for now removed features).
  • Fixed (for any bug fixes).
  • Security (for vulnerability fixes).

Explain your changes


Changed the v55.json to include the new ExternalCredentials and cloned it to create the v56.json for the new API version.

  • Jest test added to check the fix is applied.

Any particular element that can be tested locally


Any other comments


Where has this been tested?

Tested by retrieving the External Credential metadata and executing the sgd command.


Operating System: Windows

yarn version: 1.22.19

node version: v16.14.0

git version: git version 2.33.0.windows.2

sfdx version: sfdx-cli/7.175.0 win32-x64 node-v18.12.0

sgd plugin version: 5.7.1

@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (db47c1f) compared to base (9c1ad91).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #390   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           30        30           
  Lines          793       793           
=========================================
  Hits           793       793           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@scolladon scolladon left a comment

Choose a reason for hiding this comment

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

LGTM

@nodex0 Could you check if the execution of yarn increment:apiversion provide a different result ?
To do so:

$ rm -rf src/metadata/v56.json
$ yarn increment:apiversion

I tried locally and it is ok on my side, I just want to make sure it has the same output on your side as well.
Let us know the result and I'll approve the PR just after that

@scolladon
Copy link
Owner

@nodex0 I also think External Credential should be removed from v55.json
According to the documentation it is present only in v56

@codeclimate
Copy link

codeclimate bot commented Nov 12, 2022

Code Climate has analyzed commit db47c1f and detected 0 issues on this pull request.

View more on Code Climate.

@nodex0
Copy link
Contributor Author

nodex0 commented Nov 12, 2022

Removed the metadata from v55, you're right, if it was officially defined I understand it should not be there for that API version. I was a little misdirected when retrieving it with API v55, I guess it is undocumented for some older API versions.

For the yarn increment:apiversion, I am unable to run it in my Windows machine probably due to environment issues (I'm trying in mingw bash).

@scolladon
Copy link
Owner

No problem for the v.55, you were probably in a preview sandbox or preview Devhub creating preview scratch org.
That's what code review are for 😃

No problem for the yarn command as well, it is unix oriented.

Copy link
Owner

@scolladon scolladon left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for this awesome contribution, you went ahead of us and created the v56 version !

@scolladon scolladon changed the title feat: add ExternalCredentials metadata and v56 api version feat: add ExternalCredentials metadata and v56 api version Nov 12, 2022
@scolladon scolladon merged commit 5136b7a into scolladon:main Nov 12, 2022
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.

3 participants