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

Check discouraged #91

Merged
merged 4 commits into from
Dec 29, 2016
Merged

Check discouraged #91

merged 4 commits into from
Dec 29, 2016

Conversation

david-a-wheeler
Copy link
Member

No description provided.

  This adds, to the Travis checks, a check that the "discouraged"
  results are as expected.  This way, if anyone modifies the results
  (e.g., by using a theorem whose use is discouraged), it will be
  immediately noted.
  This adds a new file, "discouraged", that contains the *expected*
  results of "show discouraged" with some minor reformatting.
  It also adds a Travis check that when regenerated, the same results occur.
  The results are sorted, so that benign movement won't trigger a problem.

  If you *do* intend to change what is discouraged or the use of them,
  just regenerate the "discouraged" file using the instructions in
  .travis.yml.
@david-a-wheeler
Copy link
Member Author

Per the recent email discussion, this checks to see if the "show discouraged" results have changed.

I sort the results, so that things can move around without problems.

@nmegill
Copy link
Contributor

nmegill commented Dec 29, 2016 via email

@digama0 digama0 merged commit 7414a93 into develop Dec 29, 2016
@digama0 digama0 deleted the check-discouraged branch December 29, 2016 04:22
@digama0
Copy link
Member

digama0 commented Dec 29, 2016

@nmegill I added a new macro to mmj2, showDiscouraged.js, which produces this discouraged file in the same format as David's rearrangement of metamath.exe's output. It only takes a fraction of a second to run; perhaps this should be used instead of metamath's implementation. (Either one will produce the same result, byte-for-byte, so they can be freely interchanged.)

@david-a-wheeler
Copy link
Member Author

It takes around 20s torun locally. Not the slowest step, but noticeable. I can switch to mmj2.

@digama0
Copy link
Member

digama0 commented Dec 29, 2016

@david-a-wheeler I added an extra line RunMacro,showDiscouraged,..\..\mm\discouraged to my RunParms file, so that I am always generating an up to date discouraged file (since I often need to change discouraged theorems). If you use mmj2 you can use this to make continually regenerating the file easy. (The current run time for showDiscouraged is about 3.3 s.) Another trick you can use is to put the RunMacro line after RunProofAsstGUI, so that it doesn't slow the startup time.

@david-a-wheeler
Copy link
Member Author

How do you invoke showDiscouraged? Alternatively, can you just midify .travis.yml to use it?? Please leave a comment behind on how to do it with metanath.exe.

@digama0
Copy link
Member

digama0 commented Dec 29, 2016

Already done, see 7414a93...88a943f . Basically, you add the line

RunMacro,showDiscouraged

to your RunParms, as mentioned above; there is an optional argument which controls the output folder and filename. It is already sorted and the wording matches metamath.exe, so it can be sent straight to diff in Travis.

@nmegill
Copy link
Contributor

nmegill commented Dec 30, 2016 via email

@nmegill
Copy link
Contributor

nmegill commented Dec 30, 2016 via email

@digama0
Copy link
Member

digama0 commented Dec 30, 2016 via email

@nmegill
Copy link
Contributor

nmegill commented Dec 30, 2016 via email

@david-a-wheeler
Copy link
Member Author

One thing to think about is changing the "diff -u" to "diff -U 0" to reduce irrelevant lines in the diff, since each line is independent and the surrounding lines are just "noise" because they do not represent any particular context.

The "-u" is useful if someone is thinking about adding the lines "by hand", but in retrospect, that's extremely unlikely. I think we should do that.

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