Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

Url config minimal patch #399

Merged
merged 16 commits into from
Oct 17, 2022
Merged

Url config minimal patch #399

merged 16 commits into from
Oct 17, 2022

Conversation

JoranAngevaare
Copy link
Contributor

@coveralls
Copy link

coveralls commented Oct 13, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 82.529% when pulling 3abe4a7 on url_config_minimal_patch into 0f83fc1 on master.

jmosbacher
jmosbacher reviewed Oct 13, 2022
View reviewed changes
Copy link
Contributor

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this! just wondering if nveto to_pe is some special case here?

wfsim/strax_interface.py Outdated Show resolved Hide resolved
@JoranAngevaare JoranAngevaare marked this pull request as ready for review October 13, 2022 16:41
jmosbacher
jmosbacher approved these changes Oct 13, 2022
View reviewed changes
Copy link
Contributor

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

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

A good example of "don't let perfect be the enemy of good". It solves the problem :)

@jmosbacher
Copy link
Contributor

@JoranAngevaare looks like the test is the issue here

@JoranAngevaare
Copy link
Contributor Author

thx 👍 !

@JoranAngevaare
Copy link
Contributor Author

I'll remove the scaffolding and merge after straxen 2.0.0 is released

@JoranAngevaare JoranAngevaare merged commit 2752278 into master Oct 17, 2022
@JoranAngevaare JoranAngevaare deleted the url_config_minimal_patch branch October 17, 2022 16:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jmosbacher jmosbacher jmosbacher 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.
3 participants
@JoranAngevaare @coveralls @jmosbacher