-
Notifications
You must be signed in to change notification settings - Fork 273
Stop attempting to convert previously-converted doc headers #1005
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good though I think one line is little confusing - will try it out anyway and report back.
scripts/reformat_docs.py
Outdated
if header_formatter.needs_new_header(file_contents) and converted: | ||
return '%s%s' % ( | ||
block_contents.group(0), | ||
header_formatter.convert(header_from_block(block)) + '\n') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I am misunderstanding something, can we not use converted
here. Why do we now need the check converted
is truthy? Surely if it is an empty string this amounts to returning block_contents.group(0)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well spotted, I'll fix that now.
Should be fixed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - but I will test it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - but I will test it works for my use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works - the approach that works for me is to do a rebase on the current base (i.e not master
) first. I will document this on the wiki and share on slack.
When this is merged this documentation can be simplified to just say "grab latest version of the script": https://github.com/diffblue/doc-diffblue-common/wiki/Rebasing-PRs-after-the-Doxygen-Change |
When I try to run the script as a stand-alone program (to do help update a branch) it seems to hang:
|
Interesting - this suggests that it doesn't like namedtuple for some reason, but that's been in Python since v2.4. Can you open an interpreter and run import collections
Field = collections.namedtuple('Field', ['name', 'contents']) |
Ignore what I said above. The script is not designed to be run directly (which is why it was not marked executable). If you want to run it in that way, you should add |
I updated the wiki page with a bit more info about actually running the conversion scripts. |
On Thu, 2017-06-15 at 07:41 -0700, Reuben Thomas wrote:
I updated the wiki page with a bit more info about actually running the conversion scripts.
Appreciated. We should really send out a version of this on
cprover-announce and / or cprover-support as this will break a lot of
external folk's code.
|
Updates the documentation conversion script so that it doesn't try to convert file headers which have already been converted. This should mean that, after the initial run, subsequent runs of the
do_doc_convert
script do not make any changes to the codebase, which should make rebases from pre-Doxygen code much easier.