Skip to content

feat: add event parameter ordering#26

Open
abdulla-cb wants to merge 2 commits into
mainfrom
abdulla/event-parameter-ordering
Open

feat: add event parameter ordering#26
abdulla-cb wants to merge 2 commits into
mainfrom
abdulla/event-parameter-ordering

Conversation

@abdulla-cb

Copy link
Copy Markdown
  • add rule about ordering event parameters

@cb-heimdall

cb-heimdall commented Sep 9, 2024

Copy link
Copy Markdown
Collaborator

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

Comment thread README.md Outdated
event UpdatedOwner(address newOwner);
```

##### C. Order event parameters in chronological order.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think "chronological" could be confusing 🤔 . Might make more sense to say like, "Events logging updated values should follow the convention: previous, new

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah I agree I think it's confusing, I've just updated it - thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants