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

Change the definition of is_interval to rely on large inequalities #411

Merged
merged 3 commits into from
Sep 28, 2021

Conversation

ybertot
Copy link
Member

@ybertot ybertot commented Jul 29, 2021

The previous definition, based on strict inequalities, is less practical

The impact of this change has been tested against two other pull requests that use the is_interval concept

@CohenCyril
Copy link
Member

Hi @ybertot, I do not understand what 793badc are 3aa4fe5 doing here. Did I miss something?

@ybertot
Copy link
Member Author

ybertot commented Jul 30, 2021

I don't understand either. I did a rebase, and all of a sudden these commits were included in the branch. Maybe I goofed one of my rebase, and then I had to do force push, but people usually have to do a force push after a rebase, so that you are really working without a safety net at this point.

Maybe the solution will be to remake the branch from scratch and cherry-pick the commits. I am not too fluent, but I may be able to repair that.

@CohenCyril
Copy link
Member

I did a rebase, and all of a sudden these commits were included in the branch.

Well, it looks like you rebased master on top of is_itv and then added a commit...

@ybertot
Copy link
Member Author

ybertot commented Jul 30, 2021

should be fixed now.

Copy link
Member

@CohenCyril CohenCyril left a comment

Choose a reason for hiding this comment

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

OK, I'm 100% of the benefits of this change.
@affeldt-aist do you agree?

@affeldt-aist
Copy link
Member

OK, I'm 100% of the benefits of this change.
@affeldt-aist do you agree?

Yes, no problem.

Copy link
Member

@CohenCyril CohenCyril left a comment

Choose a reason for hiding this comment

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

nitpicking

theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
theories/normedtype.v Outdated Show resolved Hide resolved
Copy link
Member

@CohenCyril CohenCyril left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Cyril Cohen <[email protected]>
@affeldt-aist affeldt-aist merged commit f8eff34 into math-comp:master Sep 28, 2021
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.

3 participants