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

chore/remove class files, fix typos, code refactor and sonar findings #89

Merged
merged 14 commits into from
Apr 18, 2024
Merged

chore/remove class files, fix typos, code refactor and sonar findings #89

merged 14 commits into from
Apr 18, 2024

Conversation

nitin-vavdiya
Copy link
Contributor

@nitin-vavdiya nitin-vavdiya commented Feb 26, 2024 •
edited
Loading

Description

  1. Class files removed
  2. Typos fixed in exception classes
  3. Java doc fixed in exception classes
  4. code refactor
  5. Sonar finding

Before sonar fixes:
Screenshot 2024-02-26 at 12 00 20 PM

After sonar fixes:
Screenshot 2024-02-26 at 12 38 24 PM

Pre-review checks

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

@nitin-vavdiya nitin-vavdiya changed the title Chore/remove class files Chore/remove class files, fix typos and code refactor Feb 26, 2024
@nitin-vavdiya nitin-vavdiya changed the title Chore/remove class files, fix typos and code refactor Chore/remove class files, fix typos, code refactor and sonar findings Feb 26, 2024
@nitin-vavdiya
Copy link
Contributor Author

nitin-vavdiya commented Feb 26, 2024 •
edited
Loading

@borisrizov-zf Can you re-run the build job, not sure why it is failing.

@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya seems that the dash license tool has some issues right now.

@nitin-vavdiya
Copy link
Contributor Author

@nitin-vavdiya seems that the dash license tool has some issues right now.
Thanks for checking out.
Can we re-run today and check if the issue is resolved?

@nitin-vavdiya
Copy link
Contributor Author

@borisrizov-zf May I know what is the blocker to merge this PR?

@borisrizov-zf
Copy link
Contributor

borisrizov-zf commented Mar 21, 2024 •
edited
Loading

@nitin-vavdiya I was actually hoping to merge the #77 first. It's just taking some time.

@nitin-vavdiya nitin-vavdiya mentioned this pull request Mar 26, 2024
Open
borisrizov-zf
borisrizov-zf requested changes Apr 2, 2024
View reviewed changes
src/main/java/org/eclipse/tractusx/ssi/examples/BuildDIDDocJsonWebKey2020.java
@@ -30,8 +30,8 @@
import org.eclipse.tractusx.ssi.lib.crypt.IPrivateKey;
import org.eclipse.tractusx.ssi.lib.crypt.IPublicKey;
import org.eclipse.tractusx.ssi.lib.crypt.jwk.JsonWebKey;
import org.eclipse.tractusx.ssi.lib.crypt.x25519.x25519PrivateKey;
import org.eclipse.tractusx.ssi.lib.crypt.x25519.x25519PublicKey;
import org.eclipse.tractusx.ssi.lib.crypt.x25519.X25519PrivateKey;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change, I'm not sure it's necessary, just because sonar is squawking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to align with Java naming convention

src/main/java/org/eclipse/tractusx/ssi/examples/BuildDIDDocEd25519VerificationKey2020.java Show resolved Hide resolved
src/main/java/org/eclipse/tractusx/ssi/examples/ResolveDIDDoc.java Outdated Show resolved Hide resolved
src/main/java/org/eclipse/tractusx/ssi/examples/VC.java Outdated Show resolved Hide resolved
src/main/java/org/eclipse/tractusx/ssi/examples/VC.java Outdated Show resolved Hide resolved
src/main/java/org/eclipse/tractusx/ssi/lib/model/verifiable/Verifiable.java
@@ -57,7 +57,7 @@ public enum VerifiableType {
* @param json the json
* @param type the type
*/
public Verifiable(Map<String, Object> json, VerifiableType type) {
protected Verifiable(Map<String, Object> json, VerifiableType type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the visibility changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

src/main/java/org/eclipse/tractusx/ssi/lib/proof/IVerifier.java Outdated Show resolved Hide resolved
src/main/java/org/eclipse/tractusx/ssi/lib/proof/IVerifier.java Outdated
@@ -39,18 +38,16 @@ public interface IVerifier {
* model to get the public key of issuer.
*
* @param hashedLinkedData the hashed linked data
* @param document {@link VerifiableCredential} the verifiable
* @param verifiable {@link VerifiableCredential} the verifiable
* @return boolean if verified or not
* @throws UnsupportedSignatureTypeException
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

verifiable is a method argument of verify method

src/test/java/org/eclipse/tractusx/ssi/lib/proof/SignAndVerifyTest.java Outdated Show resolved Hide resolved
src/test/java/org/eclipse/tractusx/ssi/lib/proof/SignAndVerifyTest.java Outdated Show resolved Hide resolved
@nitin-vavdiya
Copy link
Contributor Author

@borisrizov-zf Class rename is only to align with Java naming standard

borisrizov-zf
borisrizov-zf approved these changes Apr 12, 2024
View reviewed changes
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.

Please rebase, as we've merged a PR since the last edit. I'll merge after.

@nitin-vavdiya nitin-vavdiya force-pushed the chore/remove-class-files branch from 39c8365 to 9591bec Compare April 12, 2024 09:19
@nitin-vavdiya
Copy link
Contributor Author

@borisrizov-zf rebase done

@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya hi, I checked the rebase-ability of the branch against #91 . They have merge conflicts, so please wait for the merge and rebase. While you're rebasing, please also fix typos in the git commits such as in ec99f9d11e87d4e4af35f6e3717ef53e752fc7e4 where you've typed chroe instead of chore. I'll ping you when you can start rebasing.

@nitin-vavdiya nitin-vavdiya changed the title Chore/remove class files, fix typos, code refactor and sonar findings chore/remove class files, fix typos, code refactor and sonar findings Apr 17, 2024
@nitin-vavdiya nitin-vavdiya force-pushed the chore/remove-class-files branch from a029a92 to 81408ff Compare April 17, 2024 04:41
@borisrizov-zf
Copy link
Contributor

@nitin-vavdiya Hi, the JWT branch is merged. You can go ahead and rebase. You'll get some issues, but it's mostly updated docstrings.

nitin-vavdiya and others added 9 commits April 17, 2024 14:18
@nitin-vavdiya
xfix: eclipse-tractusx#70
2f07229
@nitin-vavdiya
chroe: typos, code refactor and class files removed
7717901
@nitin-vavdiya
fix: sonar findings
462a1ce
@nitin-vavdiya
fix: spotless
a8d0662
@nitin-vavdiya
fix: imports
c699031
@nitin-vavdiya
code changes as per review
ea40398
@nitin-vavdiya @borisrizov-zf
Update src/main/java/org/eclipse/tractusx/ssi/examples/ResolveDIDDoc.…
9749c12 
…java

Co-authored-by: Boris Rizov (ZF Friedrichshafen AG) <138589018+borisrizov-zf@users.noreply.github.com>
@nitin-vavdiya @borisrizov-zf
Update src/test/java/org/eclipse/tractusx/ssi/lib/proof/SignAndVerify…
04aad23 
…Test.java

Co-authored-by: Boris Rizov (ZF Friedrichshafen AG) <138589018+borisrizov-zf@users.noreply.github.com>
@nitin-vavdiya @borisrizov-zf
Update src/test/java/org/eclipse/tractusx/ssi/lib/proof/SignAndVerify…
b6b5206 
…Test.java

Co-authored-by: Boris Rizov (ZF Friedrichshafen AG) <138589018+borisrizov-zf@users.noreply.github.com>
@nitin-vavdiya nitin-vavdiya force-pushed the chore/remove-class-files branch from 81408ff to c1a86a5 Compare April 17, 2024 08:56
@nitin-vavdiya
Copy link
Contributor Author

@borisrizov-zf Rebase done

@borisrizov-zf borisrizov-zf merged commit e92380a into eclipse-tractusx:main Apr 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borisrizov-zf borisrizov-zf borisrizov-zf approved these changes

@DominikPinsel DominikPinsel DominikPinsel approved these changes

Assignees

@nitin-vavdiya nitin-vavdiya

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nitin-vavdiya @borisrizov-zf @DominikPinsel