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

pyvroom: expose speed_factor and per_hour #901

Merged
merged 4 commits into from
Mar 16, 2023
Merged

pyvroom: expose speed_factor and per_hour #901

merged 4 commits into from
Mar 16, 2023

Conversation

jonathf
Copy link
Collaborator

@jonathf jonathf commented Mar 11, 2023

I'm updating pyvroom to get feature pairity with version 1.13. To get there I need to expose the speed_factor attribute.

I'm also exposing per_hour, thought I do have it indirectly in other context, I think it is much cleaner if it is exposed it directly aswell.

@jonathf jonathf requested a review from jcoupey March 11, 2023 14:18
@jcoupey
Copy link
Collaborator

jcoupey commented Mar 14, 2023

No problem on the principle of exposing those two. My only concerns are:

  • I'm reluctant to tag a patch release only for a "convenience" change for downstream use;
  • if we merge this into the release/1.13 branch, we'd have to also apply it to master.

What do you think if we do it the other way around: we merge those changes into current master first to have them in main line, then I simply cherry-pick what's required to the release/1.13 branch. Maybe we don't even have to tag a release if you aim at the tip of that branch from downstream?

@jonathf jonathf changed the base branch from release/1.13 to master March 15, 2023 08:13
@jonathf
Copy link
Collaborator Author

jonathf commented Mar 15, 2023

Thats fine.

I've rebased the branch and repointed the PR towards master now.

I also recompiled pyvroom against the current master, and it works the same as the release. I have no problem pointing pyvroom to the current tip.

@jcoupey jcoupey merged commit 0174abd into master Mar 16, 2023
@jcoupey
Copy link
Collaborator

jcoupey commented Mar 16, 2023

I just cherry-picked the changes from this branch to release/1.13 in e2653a4.

@jcoupey jcoupey deleted the pyvroom-1.13 branch March 16, 2023 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants