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.

Sign up for GitHub

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

Jump to bottom

Update EIP-3475: Changing comments #5789

Merged
merged 23 commits into from
Jan 24, 2023
Merged

Update EIP-3475: Changing comments #5789

merged 23 commits into from
Jan 24, 2023

Conversation

dhruvmalik007
Copy link
Contributor

description defined in the interface file.

dhruvmalik007 and others added 16 commits September 26, 2022 13:23
@dhruvmalik007
some minor description changes in interface and example implemenation.
f3d39dc
@dhruvmalik007
rectifying description of the array struct.
6fd0a6b
@dhruvmalik007 @Pandapip1
Update assets/eip-3475/interfaces/IERC3475.sol
70b7d1c 
grammer refactor

Co-authored-by: Pandapip1 <[email protected]>
@dhruvmalik007 @Pandapip1
Update assets/eip-3475/interfaces/IERC3475.sol
18b3a81 
Co-authored-by: Pandapip1 <[email protected]>
@dhruvmalik007 @Pandapip1
Update assets/eip-3475/interfaces/IERC3475.sol
6c99965 
Co-authored-by: Pandapip1 <[email protected]>
@dhruvmalik007 @Pandapip1
Update assets/eip-3475/interfaces/IERC3475.sol
32320d7 
refactor

Co-authored-by: Pandapip1 <[email protected]>
@dhruvmalik007
refactoring and addressing ambiguous descriptions
d958901
@dhruvmalik007
Merge branches and EIP review changes.
9727fde
@dhruvmalik007
refactor
dae0849
@dhruvmalik007
Merge branch 'ethereum:master' into dhruvmalik007/changing-comments-3475
b6b5e8d
@dhruvmalik007
refactor final
a42e07f
@dhruvmalik007
adding correct syntax and all refactoring done !!
600ae6b
@dhruvmalik007
spaces and refactoring
cc86cef
@dhruvmalik007 @Pandapip1
update
18437ab 
Co-authored-by: Pandapip1 <[email protected]>
@dhruvmalik007
adding the comments in the interfaces from the standard description.
4ff1ba8
@dhruvmalik007
sync local branch
adacdf2
@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Oct 17, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Oct 17, 2022 •
edited
Loading

A critical exception has occurred:
Message: pr 5789 is already merged; quitting
(cc @alita-moore, @mryalamanchi)

@SamWilsn
Copy link
Contributor

Could you explain the changes you're making to this EIP? It's in the final state, so generally we don't allow updates.

@dhruvmalik007
Copy link
Contributor Author

Could you explain the changes you're making to this EIP? It's in the final state, so generally we don't allow updates.

hi @SamWilsn , we wanted to rectify the description of the example information that is defined in the interface as many developers were having hard time understanding the notations like [1,10,00] storage of the Transaction tuple, which is shown as the pseudo-code , but other readers who were developing on the standard were getting errors.

so the changes are only in the descriptions and never change either the function interface or standard

@Pandapip1 Pandapip1 changed the title Dhruvmalik007/changing comments 3475 Update EIP-3475: Changing comments Oct 23, 2022
@dhruvmalik007 dhruvmalik007 dismissed a stale review via 85a25dc October 28, 2022 17:16
@SamWilsn SamWilsn dismissed stale reviews from ghost via 85a25dc October 28, 2022 17:56
@eth-bot eth-bot dismissed a stale review via 85a25dc October 31, 2022 11:52
Pandapip1
Pandapip1 previously approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

These are non-normative changes, so this is fine.

@eth-bot eth-bot dismissed stale reviews from ghost via 2c681d1 January 17, 2023 02:23
@github-actions
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the w-stale Waiting on activity label Jan 24, 2023
@dhruvmalik007
Copy link
Contributor Author

ah sorry didnt checked this PR issue, ok I will close this one and re-create a new PR.

@eth-bot eth-bot dismissed stale reviews from ghost via 2c681d1 January 24, 2023 14:12
Pandapip1
Pandapip1 approved these changes Jan 24, 2023
View reviewed changes
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

I believe this is fine. These do not change the spec.

@Pandapip1
Copy link
Member

ah sorry didnt checked this PR issue, ok I will close this one and re-create a new PR.

No need. Not sure why this didn't merge the 1st time.

@eth-bot eth-bot enabled auto-merge (squash) January 24, 2023 14:14
@eth-bot eth-bot merged commit 8d864ed into ethereum:master Jan 24, 2023
iseriohn pushed a commit to iseriohn/EIP-NFT-Rights-Management that referenced this pull request Feb 16, 2023
@dhruvmalik007 @Pandapip1 @iseriohn
Update EIP-3475: Changing comments (ethereum#5789)
445c722 
* some minor description changes in interface and example implemenation.

* rectifying description of the array struct.

* Update assets/eip-3475/interfaces/IERC3475.sol

grammer refactor

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

* Update assets/eip-3475/interfaces/IERC3475.sol

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

* Update assets/eip-3475/interfaces/IERC3475.sol

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

* Update assets/eip-3475/interfaces/IERC3475.sol

refactor

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

* refactoring and addressing ambiguous descriptions

* refactor

* refactor final

* adding correct syntax and all refactoring done !!

* spaces and refactoring

* update

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

* adding the comments in the interfaces from the standard description.

* separating the examples for events, decluttering.

* updating  the example class name

* removing redundant metadata description.

* refactor the standard based on feedback  PR ethereum#6128

* add example

* modified:   EIPS/eip-3475.md
- Rectifying the example on explaining the approval function(L#126).
- Changing the formatting of subheadings in rational to be better visiblity and linting. (L#268).
- Rectifying the definition of `event Redeem()` to include transaction struct parameter. (L#297).
- Linting suggestion(L#346,L#385, L#404).
- Re-writing the sentence that defines the rational of having metadata structure(L#365)

	modified:   assets/eip-3475/ERC3475.sol
- clarifying the correct definition of value in metadata.

Co-authored-by: Pandapip1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SamWilsn SamWilsn SamWilsn left review comments

@xinbenlv xinbenlv xinbenlv left review comments

@eth-bot eth-bot eth-bot approved these changes

@Pandapip1 Pandapip1 Pandapip1 approved these changes

Assignees
No one assigned
Labels
c-update Modifies an existing proposal s-final This EIP is Final t-erc w-stale Waiting on activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dhruvmalik007 @eth-bot @SamWilsn @Pandapip1 @xinbenlv