-
Notifications
You must be signed in to change notification settings - Fork 1
[GPCAPIM 247] PDS access module and stub #38
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
Conversation
davidhamill1-nhs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this is an MVP, the only real blocker of concern is that PDS retrievals (Get a patient's record) do not return a Bundle, but rather a Patient resource.
It doesn't look like there are coding standards yet laid out for this project, but personally I lean away from comments and towards code that explains itself, saving docstrings, etc. to only explain the why not the what. And given this is only a starting point for the stub, they would quickly need updating or more likely go out of date.
With that said, most of my comments are nit picks, and given I am new to the team, I am happy to cede to the group's opinion.
Happy to have a chat through any of it tomorrow afternoon.
|
@davidhamill1-nhs It's funny you should say that, I was having a conversation with @DWolfsNHS about comments vs. "the code is the documentation" just yesterday. My own (quite strong) preference is for having docstrings on everything as a bare minimum and then adding more comments in the actual code, because I've been bitten more than once by code that was supposedly self-explanatory that actually was just an excuse for the person who wrote it not to have to write documentation. My feeling is that if code were self-explanatory there wouldn't have been so many mechanisms invented for documenting and explaining it. 😆 That said, I'm fine with going along with the majority if the rest of the team disagrees! |
b34ead8 to
d6bbf8b
Compare
|
@davidhamill1-nhs @DWolfsNHS Review comments addressed, if I could get a 👍 so I can merge it please? (@davidhamill1-nhs do you have write access yet? If you don't then your 👍 is welcome but I'll also need one from someone else!) |
|
Right, Sonar placated (also coverage fixed). @DWolfsNHS Any chance of a 👍 please? |
329f532 to
29cd915
Compare
…tecture - Trim trailing hyphens from the target group name - Change ECS task definition CPU architecture from ARM64 to X86_64
|
|
Right, think I'm there now. @DWolfsNHS Could I get a (hopefully final) 👍 please? @BJSS-russell-pollock it looks to be there from a testing perspective as well, please do satisfy yourself of that! |
|
QA pass from @BJSS-russell-pollock |




Description
Creates the PDS access module pursuant to https://nhsd-jira.digital.nhs.uk/browse/GPCAPIM-247. Also creates the stub PDS class so that tests can be run against it and adds the stubs directory to the python environment for the dev container.
Context
PDS access module is an integral part of the clinical gateway system, required to call out to PDS in order to obtain the ODS code for the patient's current GP. The stub is required to permit testing so that we don't have to call the actual PDS system in unit tests.
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.