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(openssl): version upgrade to v0.10.48 in cactus-plugin-keychain-vault #2403

Closed
wants to merge 4 commits into from
Closed

Conversation

Poonam1607
Copy link
Contributor

…tements in cactus-plugin-keychain-vault package

…tements in cactus-plugin-keychain-vault package
@Poonam1607 Poonam1607 changed the title Update OpenSSL version and remove unnecessary parentheses from if sta… refractor: Update OpenSSL version and remove unnecessary parentheses from if sta… Apr 24, 2023
@Poonam1607 Poonam1607 changed the title refractor: Update OpenSSL version and remove unnecessary parentheses from if sta… refactor: Update OpenSSL version and remove unnecessary parentheses from if sta… Apr 24, 2023
@jagpreetsinghsasan jagpreetsinghsasan changed the title refactor: Update OpenSSL version and remove unnecessary parentheses from if sta… feat(openssl): version upgrade Apr 24, 2023
Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

@Poonam1607 thankyou for the PR
As far I can see in the changed files, there is no change of parenthesis anywhere.
So I am updating the PR title.
For the PR description and the commit message, please refer to our contribution guide.
You can also refer to one of my merged PR's for reference: https://github.com/hyperledger/cacti/pull/2272

@Poonam1607
Copy link
Contributor Author

@Poonam1607 thankyou for the PR As far I can see in the changed files, there is no change of parenthesis anywhere. So I am updating the PR title. For the PR description and the commit message, please refer to our contribution guide. You can also refer to one of my merged PR's for reference: #2272

Thankyou @jagpreetsinghsasan for doing that! Also, is there anything else wrong with the commit? Some checks are failing, can you please review it?

@jagpreetsinghsasan
Copy link
Contributor

@Poonam1607 the previous review comments I provided are still not fixed.
The DCO check is failing as you probably haven't signed off the commit.
To fix it you need to add the signed commit as git commit -s -m ..

Signed-off-by: Poonam1607 <[email protected]>
@sandeepnRES
Copy link
Contributor

sandeepnRES commented May 2, 2023

@Poonam1607 Hi, Thank you for the PR, but there are few issues with PR, so it can't be merged yet.

I am updating the title again just to make it more unique.
Can you create only one commit with the same changes, and use the PR title as commit message and also use git commit -s to sign off the commit.

Also squashing the commit is not possible, as there are some conflicts, so you will have to create same changes again in fresh branch/fork that is in sync with main. Make sure there are no merge commits in the PR.

@sandeepnRES sandeepnRES changed the title feat(openssl): version upgrade feat(openssl): version upgrade to v0.10.48 in cactus-plugin-keychain-vault May 2, 2023
@Poonam1607
Copy link
Contributor Author

@Poonam1607 Hi, Thank you for the PR, but there are few issues with PR, so it can't be merged yet.

I am updating the title again just to make it more unique. Can you create only one commit with the same changes, and use the PR title as commit message and also use git commit -s to sign off the commit.

Also squashing the commit is not possible, as there are some conflicts, so you will have to create same changes again in fresh branch/fork that is in sync with main. Make sure there are no merge commits in the PR.

I did the changes with a new PR - feat(openssl): version upgrade to v0.10.48 in cactus-plugin-keychain-vault #2412

@Poonam1607 Poonam1607 closed this by deleting the head repository May 4, 2023
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