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

Conversion of package appverifier > readercertgen to Kotlin #397

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Conversion of package appverifier > readercertgen to Kotlin #397

merged 2 commits into from
Oct 30, 2023

Conversation

keesgeluk
Copy link
Contributor

Fixes #TBD (conversion to Kotlin, second intent to create this pull request...)

It's a good idea to open an issue first for discussion.

  • [x ] Tests pass

Conversion of package appverifier > readercertgen to Kotlin

Copy link
Contributor

@mitrejcevski mitrejcevski 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! I only added some styling/formatting remarks, but nothing serious


// Extensions --------------------------
val jcaX509ExtensionUtils: JcaX509ExtensionUtils
jcaX509ExtensionUtils = try {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be defined into a single expression, like so:

val jcaX509ExtensionUtils = try { 
  ... 
} catch { 
  ... 
}

or even

the type will be inferred automatically

return try {
// NOTE older devices may not have the right BC installed for this to work
val kpg: KeyPairGenerator
if (curve.equals("Ed25519", ignoreCase = true) || curve.equals(
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the formatting of this if can be improved, as it is a bit tricky to read it this way 🤔
I'd move the expression from || onwards in the line below, something like

if (curve.equals("Ed25519", ignoreCase = true) 
    || curve.equals("Ed448", ignoreCase = true)
) {
  ....   
}    

Copy link
Contributor

@mitrejcevski mitrejcevski 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

@davidz25 davidz25 merged commit aa50f95 into openwallet-foundation-labs:master Oct 30, 2023
2 checks passed
@keesgeluk keesgeluk deleted the conversion2kotlin branch October 31, 2023 13:03
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