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

[tests,package,#19][s]: Add test_null_values_are_stripped #26

Merged
merged 3 commits into from
Jun 18, 2020

Conversation

sglavoie
Copy link
Contributor

This is a very simple addition: it removes the keys that have the value null in a CKAN package just as it is already doing it in the same way for CKAN resources.

Note: I took the liberty to rename the test from test_null_values_are_removed to test_null_values_are_stripped so that it matches with the name of the test we have for CKAN resources.

@rufuspollock
Copy link
Contributor

@sglavoie i can't merge b/c of conflicts. Can you fix these.

Note: I took the liberty to rename the test from test_null_values_are_removed to test_null_values_are_stripped so that it matches with the name of the test we have for CKAN resources.

This is a great note. BTW you can have these in the commit messages so they get logged in the git history (makes the commits easier to understand if you do git log).

@sglavoie
Copy link
Contributor Author

sglavoie commented Jun 18, 2020

@rufuspollock, this should now be good to go! 👍

Edit: I just saw that it failed, let me have a closer look.

* Merge branch 'master' into tests/null_values_are_stripped to avoid
  conflicts from the other PR.
* `test_no_resources_return_empty_list` created a new requirement which
  broke this commit.
@sglavoie
Copy link
Contributor Author

@rufuspollock, now it's working again, ready to merge!

@rufuspollock rufuspollock merged commit 34940e6 into master Jun 18, 2020
@sglavoie sglavoie deleted the tests/null_values_are_stripped branch July 7, 2020 15:19
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