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

Release/0.4.0 #66

Merged
merged 3 commits into from
Jul 19, 2021
Merged

Release/0.4.0 #66

merged 3 commits into from
Jul 19, 2021

Conversation

colmsnowplow
Copy link
Collaborator

@colmsnowplow colmsnowplow commented Jul 13, 2021

This is the release PR for the first addition of filtering.

It contains all of the changes from #60, squashed into one commit, as well as fixes for lint issues across the project.

Apologies for needing to create a new PR, I forgot I wasn't on a release branch. Hoping it's easy enough to approve since the difference is minimal.

The only thing to call out is that I changed some previously exported methods to be unexported in order to appease the linter - I think this doesn't cause problems but not 100% certain.

Release TODO (once any change requests are resolved):

  • Identify and carry out testing required for production release (release rc if needed)
  • Change date in CHANGELOG

Copy link
Contributor

@paulboocock paulboocock left a comment

Choose a reason for hiding this comment

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

LGTM, I don't see much need to load test here as nothing changes for existing paths. It seems unlikely that filtering will pose any negative performance issues (famous last words) but if anything unexpected was to occur then we should be able to patch as we go pretty quickly. Particularly as this is the first version of filtering that is likely to evolve over the coming months I don't feel it's worth much further exploration around its performance characteristics.

@colmsnowplow
Copy link
Collaborator Author

@paulboocock I agree on load testing - is there any other kind of testing for filtering that we think is necessary, or would you think that the unit tests I've written should suffice?

@paulboocock
Copy link
Contributor

@paulboocock I agree on load testing - is there any other kind of testing for filtering that we think is necessary, or would you think that the unit tests I've written should suffice?

I think the unit tests, along with the popular fields we expect to be used being tested manually is sufficient.

@colmsnowplow colmsnowplow removed the request for review from jbeemster July 19, 2021 10:21
@colmsnowplow colmsnowplow merged commit 68eb90e into master Jul 19, 2021
@colmsnowplow colmsnowplow deleted the release/0.4.0 branch July 19, 2021 10:28
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