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

use parameterized report approach w help of @njtierney #71

Merged
merged 24 commits into from
Apr 24, 2024

Conversation

stefaniebutland
Copy link
Member

@stefaniebutland stefaniebutland commented Oct 10, 2023

Draft PR. WIP.
See # next steps in create_certificate.R

@jules32
Copy link
Contributor

jules32 commented Oct 21, 2023

Hi @stefaniebutland , great progress! I've cleaned the code and pushed for last name only pdfs. This can be enough for this round, depending on your bandwidth. We can iterate/improve with concatenated first names for the next round.

The PDF is not displaying the hex and it could benefit from a few more carraige returns potentially <br>; not sure how much this is worth tinkering now since PDFs can be tricky; so tinker a bit but also could be fine as is; ok if no hex this first round.

Thank you!!!

image

@stefaniebutland
Copy link
Member Author

@jules32 Thanks so much for the code cleanup and pdf output!
I added participant_name (last name only) to text of certificate and that will be the version used for now. I tried adding first name too (printed correct last name plus all first names in every certificate) and realized I need to learn a little more before spending more time

@stefaniebutland
Copy link
Member Author

Status: Need to merge main into the https://github.com/Openscapes/kyber/tree/cert-completion branch, update this PR if needed, test and merge into main

@ateucher
Copy link
Contributor

ateucher commented Apr 23, 2024

@stefaniebutland I think this is ready to merge. I extracted the non-function code into a vignette and created documentation for the render_certificate() function. We could do a bit more in terms of checking inputs and add some tests, but if you're keen to get this merged now it is working, and we can address the extras later?

We could also fairly easily add the iteration inside the function so you don't need to make a loop yourself?

@stefaniebutland stefaniebutland marked this pull request as ready for review April 24, 2024 20:57
@stefaniebutland stefaniebutland merged commit ea9a42f into main Apr 24, 2024
6 checks passed
@ateucher ateucher deleted the cert-completion branch May 7, 2024 16:21
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