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

Improve AuthorizerAdaptorEntrypoint docs #2003

Merged
merged 7 commits into from
Nov 11, 2022
Merged

Improve AuthorizerAdaptorEntrypoint docs #2003

merged 7 commits into from
Nov 11, 2022

Conversation

nventuro
Copy link
Contributor

Description

This goes into much more detail as to what the issue is and how the fix works in all components involved, so that a reader that stumbles on any of them will get sufficient context to understand the full picture.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency changes
  • Code refactor / cleanup
  • Documentation or wording changes
  • Other

Checklist:

  • The diff is legible and has no extraneous changes
  • Complex code has been commented, including external interfaces
  • Tests are included for all code paths
  • The base branch is either master, or there's a description of how to merge

Copy link
Collaborator

@EndymionJkb EndymionJkb left a comment

Choose a reason for hiding this comment

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

Technically the new error message could be separate, but it is related. We have other tests for invalid data that check for "Transaction reverted without a reason"; could call it INVALID_DATA and use it for those cases too. Or say INVALID_CALLDATA / MALFORMED_CALLDATA to differentiate between explicit calldata and general encoded data.

Copy link
Contributor

@jubeira jubeira left a comment

Choose a reason for hiding this comment

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

Looks good! Just a few minor comments / questions.

@nventuro nventuro merged commit 8986e95 into master Nov 11, 2022
@nventuro nventuro deleted the auth-docs branch November 11, 2022 17:49
TomAFrench added a commit that referenced this pull request Nov 14, 2022
* master:
  Fix linter (#2006)
  Improve AuthorizerAdaptorEntrypoint docs (#2003)
  Changelog update (#2000)
dandamiangow3t pushed a commit to IlluviumGame/balancer-v2-monorepo that referenced this pull request May 10, 2023
* Add AuthorizerAdaptor docs and warnings

* Small test adjustment

* Adjust authorizer comments

* Clarify action ids matching

* Minor adaptor comment update

* Adjust entrypoint comments

* Apply suggestions from code review

Co-authored-by: EndymionJkb <[email protected]>

Co-authored-by: EndymionJkb <[email protected]>
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