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

Sdk 2235 share v 2 retrieve receipt #289

Merged
merged 39 commits into from
Dec 14, 2023
Merged

Sdk 2235 share v 2 retrieve receipt #289

merged 39 commits into from
Dec 14, 2023

Conversation

mehmet-yoti
Copy link
Contributor

No description provided.

irotech
irotech reviewed Oct 31, 2023
View reviewed changes
Copy link

@irotech irotech left a comment

Choose a reason for hiding this comment

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

could you please review the commits messages?

digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated
share.SessionID = receiptResponse.ID
share.UserContent = userContent

return share, err
Copy link

Choose a reason for hiding this comment

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

receipt or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upodated

klaidas
klaidas reviewed Nov 3, 2023
View reviewed changes
digital_identity_client.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
_examples/digitalidentity/main.go Outdated Show resolved Hide resolved
_examples/digitalidentity/main.go Outdated Show resolved Hide resolved
_examples/digitalidentity/main.go Show resolved Hide resolved
_examples/digitalidentity/main.go Outdated Show resolved Hide resolved
_examples/digitalidentity/main.go Outdated Show resolved Hide resolved
@mehmet-yoti
SDK-2254 Resolved pr comments
ae46a53
@mehmet-yoti
SDK-2259:updated needed pr reviews
22bbe7d
@mehmet-yoti
SDK-2259:updated needed pr reviews
9a619e8
@mehmet-yoti
SDK-2259:updated needed pr reviews
bc20038
@mehmet-yoti
SDK-2254 Resolved pr comments
00ef7b0
@mehmet-yoti
SDK-2254:updated pr changes
4cf2791
@mehmet-yoti
SDK-2254 removed unused builder
ad1f3f9
@mehmet-yoti
SDK-2259:added retrieve qr code structs
87d61de
@mehmet-yoti
SDK-2254 Removed unused examples and updated function namings
6c2d312
@mehmet-yoti
SDK-2259:removed unused example files
d40c218
@mehmet-yoti
SDK-2254 Removed unused examples and updated function namings
f8657c2
@mehmet-yoti
SDK-2259:removed unused example files
4c019b3
@mehmet-yoti
SDK-2235:error handling, updated comments
a841a3e
@mehmet-yoti
SDK-2235:updated method descriptions and addedd error handlers with d…
87f5098 
…escriptions
@mehmet-yoti
SDK-2235:added some error handling, updated comments, added error han…
2492915 
…dling, simplified some functions
klaidas
klaidas reviewed Nov 16, 2023
View reviewed changes
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/receipt_item_key.go Outdated Show resolved Hide resolved
klaidas
klaidas reviewed Nov 16, 2023
View reviewed changes
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
cryptoutil/crypto_utils.go Outdated Show resolved Hide resolved
_examples/digitalidentity/images/.keep Outdated Show resolved Hide resolved
_examples/digitalidentity/login.html Outdated Show resolved Hide resolved
_examples/digitalidentity/main.go Outdated Show resolved Hide resolved
klaidas
klaidas reviewed Nov 16, 2023
View reviewed changes
_examples/profile/dynamic-share.html Outdated Show resolved Hide resolved
_examples/profile/login.html Outdated Show resolved Hide resolved
consts/version.go Outdated Show resolved Hide resolved
cryptoutil/crypto_utils.go Outdated Show resolved Hide resolved
cryptoutil/crypto_utils.go Outdated Show resolved Hide resolved
mehmet-yoti and others added 2 commits November 23, 2023 14:46
@mehmet-yoti @klaidas
Update digitalidentity/service.go
5bf052f 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
9661755 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
mehmet-yoti and others added 10 commits November 23, 2023 14:47
@mehmet-yoti @klaidas
Update digitalidentity/service.go
75502ef 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
b7d7b23 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
8e20b5e 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
b893957 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
4993e35 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update cryptoutil/crypto_utils.go
e52a26c 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
b9d8fed 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update cryptoutil/crypto_utils.go
c09c2c2 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti
SDK-2235: added error details, cleaned unneccessary func and merged i…
8dd3c7c 
…nto base func. etc
@mehmet-yoti
Merge branch 'SDK-2259-go-retrieve-qr-code' into SDK-2235-share-v-2-r…
f705d44 
…etrieve-receipt
@mehmet-yoti mehmet-yoti requested a review from klaidas November 27, 2023 12:46
klaidas
klaidas reviewed Nov 27, 2023
View reviewed changes
cryptoutil/crypto_utils.go Outdated Show resolved Hide resolved
mehmet-yoti and others added 2 commits December 7, 2023 07:58
@mehmet-yoti @klaidas
Update cryptoutil/crypto_utils.go
a4775d2 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti
SDK-2235:reduced complexity
3da5372
klaidas
klaidas reviewed Dec 12, 2023
View reviewed changes
digitalidentity/service.go Outdated Show resolved Hide resolved
klaidas
klaidas reviewed Dec 12, 2023
View reviewed changes
digitalidentity/service.go Outdated Show resolved Hide resolved
klaidas
klaidas reviewed Dec 12, 2023
View reviewed changes
digitalidentity/service.go Outdated Show resolved Hide resolved
klaidas
klaidas approved these changes Dec 12, 2023
View reviewed changes
digitalidentity/service.go Outdated

responseBytes, err := io.ReadAll(response.Body)
if err != nil {
return receipt, fmt.Errorf("failed to get read response body: %v", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

all of these say get read should just be read

digitalidentity/service.go Outdated Show resolved Hide resolved
digitalidentity/service.go Outdated Show resolved Hide resolved
@klaidas
Copy link
Collaborator

klaidas commented Dec 12, 2023

Some minor mistakes in error messages that would be nice to clean up before merging, but all looks good now 👍

@mehmet-yoti mehmet-yoti requested a review from irotech December 13, 2023 13:16
irotech
irotech approved these changes Dec 13, 2023
View reviewed changes
Copy link

@irotech irotech left a comment

Choose a reason for hiding this comment

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

Looks fine from the API POV

Other concerns still stand:

  • Too many commits
  • Too generic or not relevant commits messages
  • Too many changes not related to the feature
  • Static checks and quality gates not achieved

mehmet-yoti and others added 3 commits December 14, 2023 09:50
@mehmet-yoti @klaidas
Update digitalidentity/service.go
a3d8314 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti @klaidas
Update digitalidentity/service.go
5f1ec57 
Co-authored-by: Klaidas Urbanavicius <klaidasurbanavicius@yahoo.co.uk>
@mehmet-yoti
SDK-2235 updated error context text
3fb2de7
@mehmet-yoti mehmet-yoti merged commit ac64677 into SDK-2259-go-retrieve-qr-code Dec 14, 2023
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irotech irotech irotech approved these changes

@klaidas klaidas klaidas approved these changes

@fofiuiancu fofiuiancu Awaiting requested review from fofiuiancu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mehmet-yoti @klaidas @irotech