Skip to content

Codereview digital signature #169

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

Open
lilosti opened this issue May 6, 2025 · 8 comments
Open

Codereview digital signature #169

lilosti opened this issue May 6, 2025 · 8 comments

Comments

@lilosti
Copy link

lilosti commented May 6, 2025

We have been asked to do at codereview of the digital signature PR. Any discussions about the codereview should be added as comments to this - or on the PR (depending on the nature of the discussion).

@ChatBotBerg
Copy link

@lilosti:

please provide an update / status on this issue ( in a comment) and tag @ds-bellcom in this status

please also provide

  • details regarding the review in this issue - including what's review, milestones and result
  • preference to issues identify and raised as a result of this review

I expect discussions regarding the review to be documentet in this issus and issues identifyed during the review to be raised at child issues

@lilosti
Copy link
Author

lilosti commented May 22, 2025

Hi @ChatBotBerg and @ds-bellcom,

We have started doing codereview. We have requested changes and are now awaiting Bellcom.

/Line

@ChatBotBerg
Copy link

@ds-bellcom: Please provide details regarding progress in implementing the required changes, providing me with a delivery date for digital signatur.

Best/Anna-Lis

@ds-bellcom
Copy link

Hi @lilosti and @ChatBotBerg.

I have now answered @rimi-itk question regarding #167 and are now awaiting review from ITK.

Depending on what ITK returns with after review, we expect to be able to deliver the solution approx 1-2 weeks after review.

@lilosti, will you let me know when ITK are done with the review?

@rimi-itk
Copy link
Collaborator

@ds-bellcom, a pull request with 4 failing code checks is not ready for review: https://github.com/OS2Forms/os2forms/actions/runs/14616182422

@ds-bellcom
Copy link

You may be right about that @rimi-itk. But as you write yourself, most of them should be resolved after merging #168.

We will review #168 on Wednesday, May 28th (as I have written here: #171).

@rimi-itk
Copy link
Collaborator

I'm not sure in which order you expect thing to happen, but I've added some comments on #167. Most are about inconsistencies within the new code and with the rest of the https://github.com/OS2Forms/os2forms project, and many of them could (probably) have been detected by proper code checks.

@ds-bellcom
Copy link

Hi @lilosti and @ChatBotBerg.

Stan has changed what @rimi-itk found in his review. Stan has asked @rimi-itk to make a new review. Would you please let me know when @rimi-itk has made a new review?

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

No branches or pull requests

4 participants