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

Add test for update_undocumented_columns_with_prior_knowledge function #109

Conversation

syou6162
Copy link
Contributor

@syou6162 syou6162 commented Sep 21, 2023

Continued from #96.

The update_undocumented_columns_with_prior_knowledge function has a lot of flags (skip_add_tags / skip_merge_meta / add_progenitor_to_meta) involved. Therefore, it was not clear how they behave. We have added a test to this function to make it easier to follow the behavior.

In adding testing, I also corrected some behavior that differed from the documentation (the documentation says it mutates the NODE, but in fact it does not).

@syou6162 syou6162 force-pushed the add_test_update_undocumented_columns_with_prior_knowledge branch from 66af788 to dbe9c24 Compare September 21, 2023 17:55
@syou6162 syou6162 changed the title WIP: Add test update undocumented columns with prior knowledge Add test forupdate_undocumented_columns_with_prior_knowledge function Sep 21, 2023
@syou6162 syou6162 changed the title Add test forupdate_undocumented_columns_with_prior_knowledge function Add test for update_undocumented_columns_with_prior_knowledge function Sep 21, 2023
@syou6162 syou6162 marked this pull request as ready for review September 21, 2023 18:17
@syou6162 syou6162 force-pushed the add_test_update_undocumented_columns_with_prior_knowledge branch from dbe9c24 to 672b8d6 Compare September 21, 2023 18:24
Copy link
Owner

@z3z1ma z3z1ma left a comment

Choose a reason for hiding this comment

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

Very nice. This is hugely beneficial for the sustainability of the software. 🎉
Much appreciated.

@syou6162
Copy link
Contributor Author

syou6162 commented Sep 22, 2023

@z3z1ma Thank you for reviewing & I fixed conflicted codes! Could you merge this pull request?

@z3z1ma
Copy link
Owner

z3z1ma commented Sep 22, 2023

lgtm :shipit:

@z3z1ma z3z1ma merged commit 6ff2757 into z3z1ma:main Sep 22, 2023
2 checks passed
@syou6162 syou6162 deleted the add_test_update_undocumented_columns_with_prior_knowledge branch September 22, 2023 03:55
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