Skip to content

Restrict speed_factor in the range (0, 5] #592

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 14, 2021
Merged

Restrict speed_factor in the range (0, 5] #592

merged 3 commits into from
Oct 14, 2021

Conversation

krashish8
Copy link
Contributor

@krashish8 krashish8 commented Oct 13, 2021 •
edited by jcoupey
Loading

Issue

Fix #591

Tasks

  • Add check for speed_factor range
  • Update docs/API.md
  • Update CHANGELOG.md
  • review

@krashish8
fix(src): Add check for speed_factor range
2aae597 
Signed-off-by: Ashish Kumar <ashishkr23438@gmail.com>
@krashish8
docs: Add restriction of speed_factor values
9fe6dbe 
Signed-off-by: Ashish Kumar <ashishkr23438@gmail.com>
@krashish8
chore: Update CHANGELOG.md
05b9d80 
Signed-off-by: Ashish Kumar <ashishkr23438@gmail.com>
@jcoupey jcoupey added this to the v1.11.0 milestone Oct 13, 2021
jcoupey
jcoupey approved these changes Oct 14, 2021
View reviewed changes
Copy link
Collaborator

@jcoupey jcoupey left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@jcoupey jcoupey merged commit 017af62 into VROOM-Project:master Oct 14, 2021
@jcoupey
Copy link
Collaborator

jcoupey commented Oct 14, 2021

I merged the branch in both master and release/1.11 so this will be included in the upcoming v1.11 release. Only change I performed upon merging is to remove the very specific mention on using speed_factor values with custom matrices. I feel it's enough to document the allowed range, then we could still refer to the previous discussions if one needs more context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcoupey jcoupey jcoupey approved these changes

Assignees
No one assigned
Labels
API bug
Projects
None yet
Milestone
v1.11.0
Development

Successfully merging this pull request may close these issues.

Scaling logic of speed_factor is flawed for high values
2 participants
@krashish8 @jcoupey