move
move copied to clipboard
[WIP][move-docgen] Making Docgen independent of Prover
Docgen was originally developed as a part of the Prover. However, Docgen is now independent of Prover, so moving it from Prover to its own directory seems the next logical step.
Motivation
Docgen was buried inside Prover for no apparent reason (now).
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
Unit tests pass: moved Docgen unit tests, tests for move docgen
, and all the previous tests.
Pinging my mentors @awelc and @tnowacki for review :memo:
@wrwg Sorry, I am not following here: if I understand correctly Docgen is nowadays free from Prover dependencies (docgen.rs) and can be now (#327) used directly with move docgen
without calling/using Prover?
I even experimented removing the last dev-dependencies in this commit and move docgen
compiles and works correctly. However test is still using Prover, and that's something I have to change, thank you for the pointer :+1:
It's true that Prover is still using Docgen, and hence is depending on it, but to me that seems like another tasks to remove Prover's dependence on Docgen. So to me it looks like Docgen is independent of Prover (and hence needs to be moved), and Prover is not independent of Docgen (but must be in the future).
But changing this to WIP at least because of the test refactoring (and for possible refactoring in general).
@wrwg Sorry, I am not following here: if I understand correctly Docgen is nowadays free from Prover dependencies (docgen.rs) and can be now (#327) used directly with
move docgen
without calling/using Prover?I even experimented removing the last dev-dependencies in this commit and
move docgen
compiles and works correctly. However test is still using Prover, and that's something I have to change, thank you for the pointer 👍It's true that Prover is still using Docgen, and hence is depending on it, but to me that seems like another tasks to remove Prover's dependence on Docgen. So to me it looks like Docgen is independent of Prover (and hence needs to be moved), and Prover is not independent of Docgen (but must be in the future).
But changing this to WIP at least because of the test refactoring (and for possible refactoring in general).
The prover has never been using docgen. Instead the only way to call docgen was over the prover driver (this is the same for some other tools). It is great that you point out that you made now a direct entrypoint via the Move CLI. Still can you please cut off the remaining, now unnecessary dependency? It's the one and only one AFAICT. If the docgen tests are still depending on the entry over the prover, it shouldn't be too hard to fix this as well. I'm a bit concerned about saying "next PR", because those things can be easily forgotten. It's a similar concern I had with the docgen options, where we have now some kind of duplication.
BTW for the background, docgen is a tool I've written, so I've some stake here ;-)
Thanks for trying to clean this up.
Ah and one additional remark. We where actually trying to locate all tools like docgen
in the tools
folder. Tools which are currently outside are legacy. Perhaps you can move docgen into that sub-directory. Thank you.
@wrwg Ah, okay! So, I see these two steps next for me (probably one commit each):
- Remove Prover dependency in test
- Remove Docgen code from Prover
(Current Docgen does not seem to be stand-alone, so probably it's not mature enough for tools
just yet?)
(Current Docgen does not seem to be stand-alone, so probably it's not mature enough for
tools
just yet?)
Not sure what you mean -- definitely, if it would not be mature, it would even more belong into tools instead of top-level. In general, we do not have such categories influencing the location of a crate. But there was a decision made to move all tools into the tools directory a while ago.
@wrwg Ah, sorry: at quick glance it seemed to me that tools
had only the tools which are stand-alone, usable directly from the command line. But now I see that there are some (like move-package
) which are not stand-alone. So yeah, I agree, maybe tools
is the best place for move-docgen
:+1: