Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Add LIP: "Define new transaction schema" #155

Merged
merged 35 commits into from
Sep 1, 2022
Merged

Add LIP: "Define new transaction schema" #155

merged 35 commits into from
Sep 1, 2022

Conversation

gkoumout
Copy link
Contributor

No description provided.

@gkoumout gkoumout self-assigned this Jun 21, 2022
ricott1
ricott1 suggested changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@ricott1 ricott1 left a comment

Choose a reason for hiding this comment

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

Some minor details to be fixed

proposals/lip-define_new_transaction_schema.md Outdated Show resolved Hide resolved
proposals/lip-define_new_transaction_schema.md Outdated Show resolved Hide resolved
proposals/lip-define_new_transaction_schema.md Outdated Show resolved Hide resolved
janhack
janhack suggested changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@janhack janhack left a comment

Choose a reason for hiding this comment

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

Overall, looks all good. I just left a few comments.

I did not verify the appendix as I assume that has happened already by other reviewers before the publication.

@gkoumout
Addressing comments
9646f04 
Improving/adding links, fixing typos, names of constants. Addressed all comments of Alessandro, two of Jan's comments remain to be addressed.
@gkoumout
Change motivation, move to strings
d06ba2a 
remain: Rationale and tests
@gkoumout
Update to lastest changes on names
1799913 
Appendix not updated
@janhack janhack requested a review from ishantiw August 8, 2022 12:48
@shuse2 shuse2 mentioned this pull request Aug 9, 2022
Closed
@ricott1 ricott1 requested a review from karmacoma August 16, 2022 08:14
@gkoumout gkoumout requested a review from ricott1 August 22, 2022 13:43
ricott1
ricott1 suggested changes Aug 22, 2022
View reviewed changes
Copy link
Contributor

@ricott1 ricott1 left a comment

Choose a reason for hiding this comment

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

One small comment, also please update the appendx

proposals/lip-define_new_transaction_schema.md Outdated Show resolved Hide resolved
ishantiw
ishantiw approved these changes Aug 25, 2022
View reviewed changes
Copy link
Contributor

@ishantiw ishantiw 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 small comment

proposals/lip-define_new_transaction_schema.md Outdated Show resolved Hide resolved
@karmacoma karmacoma merged commit 72b86f5 into main Sep 1, 2022
@karmacoma karmacoma deleted the add-LIP-Define-new-transaction-schema branch September 1, 2022 08:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@karmacoma karmacoma karmacoma approved these changes

@ishantiw ishantiw ishantiw approved these changes

@ricott1 ricott1 ricott1 approved these changes

@janhack janhack janhack approved these changes

Assignees

@gkoumout gkoumout

Labels
new lip
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gkoumout @karmacoma @ishantiw @ricott1 @janhack