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

A giant leap towards enforcing tools/check-typo on the entire repository #1288

Merged
merged 2 commits into from Jun 14, 2018

Conversation

Projects
None yet
3 participants
@dra27
Copy link
Contributor

commented Aug 12, 2017

Follows on from #1287, which needs merging first. The actual changes are just the last commit - 1c28da8

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2017

All the changes in this GPR relate to reducing line-lengths and fixing whitespace.

@dra27 dra27 added the suspended label Aug 13, 2017

@damiendoligez damiendoligez added this to the 4.07-or-later milestone Sep 25, 2017

@dra27 dra27 referenced this pull request Oct 11, 2017

Merged

Various changes for tools/check-typo #1287

3 of 3 tasks complete

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch from 1c28da8 to 093ae0b Oct 26, 2017

@dra27 dra27 removed the suspended label Oct 26, 2017

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch 2 times, most recently from bd5dba4 to 9777a9d Oct 26, 2017

@damiendoligez damiendoligez removed this from the consider-for-4.07 milestone Jun 5, 2018

@damiendoligez

This comment has been minimized.

Copy link
Member

commented Jun 5, 2018

@dra27 I think we should drop this PR and try again later (maybe piecewise?)

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

I don't think it needs breaking up - just rebasing (and updating).

@damiendoligez

This comment has been minimized.

Copy link
Member

commented Jun 7, 2018

But won't it be too hard to rebase?

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2018

No - I've already done it! Just testing it... it's revealed a couple of little changes to ocamltest, so once the whole testsuite is passing again, I'll split the new bits into separate pull requests and push the new version of this one just with whitespace/line-length altering changes.

@shindere

This comment has been minimized.

Copy link
Contributor

commented Jun 7, 2018

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch from 9777a9d to c00e796 Jun 8, 2018

@dra27 dra27 changed the title Enforce tools/check-typo on entire repository A giant leap towards enforcing tools/check-typo on the entire repository Jun 8, 2018

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch from c00e796 to 8228d44 Jun 8, 2018

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2018

OK, this single commit contains fixes which are solely the correction of whitespace (with occasional knock-on consequences for the testsuite reference files) and overly long lines. Overly long lines are fixed using (hopefully) non-controversial string-splitting, etc.

This diff is much easier to look at using a whitespace-eliminating diff tool such as patdiff

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 8, 2018

A clearer view of this diff may be found at http://people.ds.cam.ac.uk/dra27/gpr1288-patdiff.html

@dra27 dra27 referenced this pull request Jun 8, 2018

Merged

Tweaks for check-typo #1829

4 of 4 tasks complete

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch from 8228d44 to fc36b23 Jun 13, 2018

@dra27 dra27 force-pushed the dra27:check-typo-supreme branch from fc36b23 to a9c85ba Jun 14, 2018

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2018

Rebased, partly to kick AppVeyor, and partly because this branch should now represent a fully check-typo passing trunk

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2018

OK - assuming the revised version with GPR#1825 whitespace fixes passes, can this be merged? The patdiff-generated diff above is still correct (rebasing didn't have any conflicts).

@damiendoligez
Copy link
Member

left a comment

I have reviewed the patdiff and everything looks correct.

@dra27

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2018

Thanks, @damiendoligez! Here goes, and apologies to everyone whose patches need updating...

@dra27 dra27 merged commit 4197e75 into ocaml:trunk Jun 14, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dra27 dra27 deleted the dra27:check-typo-supreme branch Jun 14, 2018

@shindere

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.