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

No source code #7

Merged
merged 3 commits into from
May 28, 2024
Merged

No source code #7

merged 3 commits into from
May 28, 2024

Conversation

keegan-shirel
Copy link
Contributor

Description of your changes

Context described in Make source_code truly optional

Modified the generate_json_files() helper to exclude model fields that have a None value.
Modified the expected output of the unit tests to reflect excluding properties with None values.
Added a unit test to test v1 -> v3 conversion with both simple and advanced source code to validate the change worked for conversion as well.

Checklist

  • I have performed a self-review of my code
  • My code follows the contribution guidelines of this project
  • My changes generate no new warnings

@keegan-shirel
Fix ingest and conversion with no source codes
4c4ccc0
@keegan-shirel
Add unit test for converson with source code
125a28b 
Fix _convert_lineage_source when converting advanced
custom lineage v1. Was not accurately converting code
highlights.
@keegan-shirel
Copy link
Contributor Author

keegan-shirel commented May 22, 2024 •
edited
Loading

@kristofvancoillie there might've been a better way to go about it but created a new branch and pushed it up. One thing to note is a side effect of excluding anything with a None value means it will exclude props and highlights if they are null as well. I generally think it's good to exclude superfluous properties but not sure if there would be an unintended side effect.

@kristofvancoillie
Copy link
Contributor

changes look good, thx again!
Could you also update CHANGELOG.md and setup.cfg (new version)

@kristofvancoillie kristofvancoillie merged commit 54c873c into main May 28, 2024
6 checks passed
@kristofvancoillie kristofvancoillie deleted the no_source_code branch May 28, 2024 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristofvancoillie kristofvancoillie kristofvancoillie approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@keegan-shirel @kristofvancoillie