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

#87: Expand examples and add examples documentation #123

Merged
merged 4 commits into from
Mar 14, 2025

Conversation

brendanreardon
Copy link
Collaborator

@brendanreardon brendanreardon commented Mar 13, 2025

Updated all examples and wrote a README that details how each example was generated. This branch was regenerated relative to 87-add-examples because that branch was created before the updates to gks-core and VRS, and it is resulting in a many merging issues.

@brendanreardon
Copy link
Collaborator Author

Grr... some of the example validations are having issue with Variations listed under members. Looking into this...

Copy link
Contributor

@larrybabb larrybabb left a comment

Choose a reason for hiding this comment

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

This is a really nice enhancement to the docs and examples. I did not check every value, but the presentation and content of the readme looks like a good improvement.

The only cosmetic feedback I would suggest (for future) is to not use double quotes as much in the text for readers but instead use italics or bold for key values and limit the use of quoted or double-quoted terms.

Copy link
Contributor

@larrybabb larrybabb left a comment

Choose a reason for hiding this comment

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

actually, i rescind my prior approval. i just realized the tests are failing. I'll see if I can make that fix now.

@brendanreardon
Copy link
Collaborator Author

actually, i rescind my prior approval. i just realized the tests are failing. I'll see if I can make that fix now.

If you have any thoughts, I'd love to hear them! No worries either way. It seems mostly an issue with members within CategoricalCnv examples. I'm going to continue trying to troubleshoot them in the meantime.

* MSP update
* Cleaned up whitespace issues
@larrybabb larrybabb self-requested a review March 13, 2025 22:33
Copy link
Contributor

@larrybabb larrybabb left a comment

Choose a reason for hiding this comment

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

With tests passing I approve this
@brendanreardon you or @DanielPuthawala can review my fixes to the examples that helped make the tests pass and if you are okay. please merge this so I can build the Ballot branch.

@brendanreardon
Copy link
Collaborator Author

With tests passing I approve this @brendanreardon you or @DanielPuthawala can review my fixes to the examples that helped make the tests pass and if you are okay. please merge this so I can build the Ballot branch.

🤦 I can't believe that I missed that. Thank you, Larry!

@larrybabb
Copy link
Contributor

larrybabb commented Mar 14, 2025

Can I merge it?

I went for it

@larrybabb larrybabb merged commit ddafbd2 into 1.x Mar 14, 2025
13 checks passed
@larrybabb larrybabb deleted the 87-redo-examples-add-documentation branch March 14, 2025 00:19
larrybabb added a commit that referenced this pull request Mar 14, 2025
* #87: Expand examples and add examples documentation (#123)
* align to branch id

---------
Co-authored-by: Brendan Reardon <[email protected]>
Co-authored-by: James Stevenson <[email protected]>
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.

2 participants