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

Hints #1516

Merged
merged 9 commits into from
Oct 13, 2015
Merged

Hints #1516

merged 9 commits into from
Oct 13, 2015

Conversation

paf31
Copy link
Contributor

@paf31 paf31 commented Oct 11, 2015

@garyb Does this look ok? I think it simplifies things nicely and gives a nicer set of error messages.

Basically the new simplification rules is: take at most one hint from each hint category.

Thanks.

@paf31
Copy link
Contributor Author

paf31 commented Oct 11, 2015

I'd say this resolves #1357.

@paf31
Copy link
Contributor Author

paf31 commented Oct 11, 2015

This also simplifies the approach to position info slightly. Position errors always get consed to the front of the hint list, which means the most specific position data is usually at the end of the list. By reversing before picking unique hints by category, we automatically get the most specific position info.

@paf31
Copy link
Contributor Author

paf31 commented Oct 11, 2015

I don't know if this was fixed by this change or something else, but #908 is definitely working on this branch.

Resolves #908

@paf31
Copy link
Contributor Author

paf31 commented Oct 11, 2015

Resolves #909

@garyb
Copy link
Member

garyb commented Oct 12, 2015

Sounds awesome, but think this will take me a while to digest, so it'll probably be tomorrow before I get time to read it properly.

@paf31
Copy link
Contributor Author

paf31 commented Oct 13, 2015

@garyb Unless you see anything major, I'll tidy a couple of things up and merge into box-errors, then everything can be reviewed together. Sound good?

@garyb
Copy link
Member

garyb commented Oct 13, 2015

Sure thing. Code looks good to me too, I haven't tried it out yet but the idea behind it sounds solid.

paf31 added a commit that referenced this pull request Oct 13, 2015
@paf31 paf31 merged commit cb63bd4 into box-errors Oct 13, 2015
@paf31 paf31 deleted the hints branch October 13, 2015 23:44
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.

None yet

2 participants