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

NNS update rebased #420

Merged
merged 2 commits into from
Nov 12, 2024
Merged

NNS update rebased #420

merged 2 commits into from
Nov 12, 2024

Conversation

roman-khimov
Copy link
Member

It's a rebase of #266 with some additions. It largely intersects with #419, but has two additional ones. Not intended to be merged immediately.

@roman-khimov
Copy link
Member Author

After #419 merge this completely substitutes #266 with the only two changes left.

@roman-khimov roman-khimov marked this pull request as ready for review July 30, 2024 08:54
@carpawell
Copy link
Member

Rebase?

@roman-khimov
Copy link
Member Author

Doesn't matter much at this stage.

@carpawell
Copy link
Member

Is it post-release?

@roman-khimov
Copy link
Member Author

That's why it's not a part of #419, see above.

@carpawell
Copy link
Member

carpawell commented Jul 30, 2024

I see it is (was) based on #419 and has some additional changes but I do not see an explanation as to why not add it to the release. Dont mind though.

@roman-khimov
Copy link
Member Author

Because of API changes, #266 (review).

@cthulhu-rider
Copy link
Contributor

as well as i'd like to ask in #266 - pls describe motivation in the commits. Now it looks like "lets change just like that", but the behavior of the contract changes. Mb @AnnaShaleva will remind us being an original author

Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

Smth should be done here for sure.

@roman-khimov roman-khimov mentioned this pull request Aug 9, 2024
@roman-khimov roman-khimov mentioned this pull request Oct 31, 2024
@roman-khimov
Copy link
Member Author

The main motivation here is nspcc-dev/neo-go#2795. It's not related to #442/C#, btw, since C# interfaces for these methods are very much different. If we're to solve nspcc-dev/neo-go#2795 this becomes irrelevant, I think.

In case if no records of the specified type found.

Signed-off-by: Anna Shaleva <[email protected]>
And adjust method usages along the way.

Signed-off-by: Anna Shaleva <[email protected]>
Copy link
Member

@carpawell carpawell left a comment

Choose a reason for hiding this comment

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

Does not do things worse.

@carpawell carpawell merged commit 1220fc0 into master Nov 12, 2024
8 checks passed
@carpawell carpawell deleted the nns-upd-rebased branch November 12, 2024 19:18
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.

4 participants