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

fix: missing credential status in builder #90

Conversation

nitin-vavdiya
Copy link
Contributor

@nitin-vavdiya nitin-vavdiya commented Feb 26, 2024

Description

Fix for: #70

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@nitin-vavdiya
Copy link
Contributor Author

@borisrizov-zf
Can we rerun the failed job and merge?

@@ -178,6 +179,9 @@ public VerifiableCredential build() {
if (expirationDate != null) {
map.put(VerifiableCredential.EXPIRATION_DATE, formatter.format(expirationDate));
}
if (!Objects.isNull(credentialStatus)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The intended scope of Objects.isNull is in a lambda expression. This can be replaced with credentialStatus == null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@nitin-vavdiya
Copy link
Contributor Author

rebase done

Copy link
Contributor

@borisrizov-zf borisrizov-zf left a comment

Choose a reason for hiding this comment

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

Looks good. This can be merged, I'll check if it conflicts with #77 and if not we can do it first.

@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya No conflicts, so you can rebase and I'll merge after that.

@nitin-vavdiya
Copy link
Contributor Author

@nitin-vavdiya No conflicts, so you can rebase and I'll merge after that.

Rebase done

@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya Your rebase is not using upstream/main.

Follow these steps:

git config --global rerere.enable true
git fetch upstream
git checkout fix/vc-builder-credential-status-issue
git rebase upstream/main
# fix the conflict, rerere will auto-fix it for every commit after that

@nitin-vavdiya
Copy link
Contributor Author

@nitin-vavdiya Your rebase is not using upstream/main.

Follow these steps:

git config --global rerere.enable true
git fetch upstream
git checkout fix/vc-builder-credential-status-issue
git rebase upstream/main
# fix the conflict, rerere will auto-fix it for every commit after that

done

@borisrizov-zf
Copy link
Contributor

This is not correctly done:
image

Give me a call if you're having trouble or have any questions.

@nitin-vavdiya nitin-vavdiya force-pushed the fix/vc-builder-credential-status-issue branch from 44be1d5 to 4ae905d Compare March 21, 2024 09:48
@borisrizov-zf borisrizov-zf merged commit caed90f into eclipse-tractusx:main Mar 21, 2024
3 checks passed
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.

2 participants