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

Add extensions contracts to docgen #1059

Merged
merged 27 commits into from
Sep 22, 2022
Merged

Conversation

chmanie
Copy link
Member

@chmanie chmanie commented Jun 23, 2022

This PR adds the extension contracts to the docgen script. As they are not pure Interface contracts, some adjustments to the docgen script were necessary (inspection of nodes to be actual method nodes and checking of their visibility).

Note: the docgen script will now fail (exit with 1) if the docs couldn't be properly parsed in one or more instances (before they were just logged as warnings). Because we try to build the docs on CI as well now, this will make the CI tests fail if the docs can't be built.

Furthermore I fixed the issues raised in #1056 and #1057.

Closes #1056.
Closes #1057.
Closes #1036.

@chmanie chmanie force-pushed the maint/add-extension-docs branch 6 times, most recently from dc6b8e4 to 1c39ecf Compare June 23, 2022 18:12
@CLAassistant
Copy link

CLAassistant commented Jun 23, 2022

CLA assistant check
All committers have signed the CLA.

@chmanie chmanie force-pushed the maint/add-extension-docs branch 2 times, most recently from 142e5d5 to 0fb311b Compare June 23, 2022 18:40
@chmanie chmanie marked this pull request as ready for review June 24, 2022 10:16
@chmanie
Copy link
Member Author

chmanie commented Jun 24, 2022

I am not sure why now some tests are failing. Can someone with more experience look into this please?

image

@chmanie chmanie force-pushed the maint/add-extension-docs branch from 458b3ad to b339e52 Compare July 27, 2022 10:07
@chmanie chmanie force-pushed the maint/add-extension-docs branch 3 times, most recently from 89c6335 to d04e93e Compare August 9, 2022 16:03
@chmanie chmanie force-pushed the maint/add-extension-docs branch 3 times, most recently from 3882ff4 to e3f0ab6 Compare August 24, 2022 09:54
@chmanie chmanie force-pushed the maint/add-extension-docs branch 9 times, most recently from 1f7c833 to 0bea150 Compare September 7, 2022 22:29
@kronosapiens kronosapiens force-pushed the maint/add-extension-docs branch from 5208618 to fd5949e Compare September 14, 2022 16:03
kronosapiens
kronosapiens previously approved these changes Sep 14, 2022
contracts/extensions/votingReputation/VotingReputation.sol Outdated Show resolved Hide resolved
docs/guides/deploying-colony-locally.md Show resolved Hide resolved
kronosapiens
kronosapiens previously approved these changes Sep 14, 2022
@area area force-pushed the maint/add-extension-docs branch from 7d0984e to a0b1de4 Compare September 20, 2022 10:56
@area area force-pushed the maint/add-extension-docs branch from a0b1de4 to 9166810 Compare September 21, 2022 10:06
@area
Copy link
Member

area commented Sep 21, 2022

Okay, so the changes I've done here:

  • Improve the versioning script. It now considers only (functional) bytecode changes, which is what actually matters. That should mean that any doc changes going forward don't require version bumps. And indeed, I've reverted the version bumps that were in this PR as a result.

  • The downside of my changes is that checks out the old version and builds it; this makes the script inappropriate to be run in a hook, so I've removed it from there.

  • Made a test reliant on version number of an extension more resilient.

  • Reverted the one functional contract change (changing the visibility of createClaimDelayAction). I agree that should probably be done, but not here, in a doc-change PR.

Broadly speaking I'm now fine with the state of this, but I'd like @kronosapiens's input on the versioning script and if he thinks it could be improved / scenarios that it misses.

mv build build-$LATEST_RELEASE

git checkout yarn.lock
rm package-lock.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any handling if/when we move into a world where we're dealing only with the npm versions of the repo?

@kronosapiens kronosapiens merged commit 1593ff1 into develop Sep 22, 2022
@kronosapiens kronosapiens deleted the maint/add-extension-docs branch September 22, 2022 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants