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 a new env variable EXTENT_NULLABLE_FLG to allow user to customize… #616

Conversation

rzhang152
Copy link
Contributor

… extent data nullable option

Copy link
Member

@helrond helrond left a comment

Choose a reason for hiding this comment

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

Hi @rzhang152 this looks good! I just suggested some small changes to the config name and comments, mostly thinking ahead to the possibility of other configuration around inheritance of particular data attributes.

I've also been mulling whether or not it's necessary to test this particular branch of the logic, but I think it might not be worth the trouble. If you can think of a way to do it easily (like maybe with mocks and then checking the call count of specific functions) that would be great.

Once you've taken a look at those things I'll merge this to a feature branch in this repository, and from there we should be able to run tests and get the code into base.

@rzhang152 rzhang152 force-pushed the pr-pisces-extents-update branch from 8f6fd81 to c05e751 Compare September 12, 2024 22:22
@helrond helrond changed the base branch from development to inherit-extent September 13, 2024 13:32
@helrond helrond merged commit 00df7df into RockefellerArchiveCenter:inherit-extent Sep 13, 2024
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