Skip to content

feat: bump near-sdk to 5.1.0#134

Merged
karim-en merged 22 commits into
near:masterfrom
mitinarseny:fix/bump-near-sdk
May 29, 2024
Merged

feat: bump near-sdk to 5.1.0#134
karim-en merged 22 commits into
near:masterfrom
mitinarseny:fix/bump-near-sdk

Conversation

@mitinarseny
Copy link
Copy Markdown
Collaborator

No description provided.

@mitinarseny mitinarseny marked this pull request as ready for review May 27, 2024 12:30
@mitinarseny mitinarseny changed the title WIP: bump near-sdk to 5.1.0 feat: bump near-sdk to 5.1.0 May 27, 2024
Comment thread near-plugins-derive/src/access_controllable.rs Outdated
Comment thread near-plugins-derive/tests/upgradable.rs Outdated
Comment thread .gitignore
@karim-en karim-en requested a review from mooori May 27, 2024 16:38
Copy link
Copy Markdown
Contributor

@mooori mooori left a comment

Choose a reason for hiding this comment

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

To follow the v5 docs of near-sdk, I think it would be good to use #[near] instead of #[near_bindgen] in the test/example contracts.

Comment thread .github/workflows/test.yml
Comment thread .github/workflows/test.yml
Comment thread Cargo.toml Outdated
@mitinarseny mitinarseny requested a review from mooori May 29, 2024 08:55
@mooori
Copy link
Copy Markdown
Contributor

mooori commented May 29, 2024

Strangely check Rust Contracts / Tests (pull_requests) was skipped after approving workflows to run, even though relevant files where changed since the last run. It didn't tell why so I just started it manually through Github's UI.

Comment thread near-plugins-derive/src/access_controllable.rs
@mitinarseny mitinarseny requested a review from mooori May 29, 2024 13:00
Copy link
Copy Markdown
Contributor

@mooori mooori left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@mooori mooori enabled auto-merge (squash) May 29, 2024 15:55
@mooori mooori disabled auto-merge May 29, 2024 16:03
@mooori
Copy link
Copy Markdown
Contributor

mooori commented May 29, 2024

I missed 542009b was pushed while I was reviewing. Will have to take a look at that before enabling merge.

@mooori
Copy link
Copy Markdown
Contributor

mooori commented May 29, 2024

@mitinarseny what is the motivation for adding 542009b to this PR?

@mitinarseny
Copy link
Copy Markdown
Collaborator Author

mitinarseny commented May 29, 2024

I've just tried to use near-plugins in another project and it turns out that it doesn't let you use #[derive(AccessControlRole)], #[access_control], etc... without importing indirect trait dependencies into current scope:

use near_plugins::{access_control, AccessControlRole, AccessControllable, Ownable, Pausable};

#[derive(AccessControlRole, Clone, Copy)]
pub enum Role {
    // ...
}

#[access_control(role_type(Role))]
// #[derive(Ownable)], etc...
pub struct Contract {
    // ...
}

But I agree that it's not directly connected to "bumping near-sdk version".
@mooori Do you think it would be better to fix it in a separate PR?

PS: I might need to rebase the whole branch to have my commits signed... Do you mind if I do it now and re-request your review? Sorry for that, didn't pay attention in the first place
image

@karim-en
Copy link
Copy Markdown
Collaborator

PS: I might need to rebase the whole branch to have my commits signed... Do you mind if I do it now and re-request your review? Sorry for that, didn't pay attention in the first place

I want to avoid the force push and re-review, so I've disabled this requirement.

@mitinarseny
Copy link
Copy Markdown
Collaborator Author

Thank you so much! @mooori @karim-en 🙏🏻
Waiting for merge from someone with write access

PS: Already set up everything related to commit signing, so wouldn't mess up in the next PR😅

@karim-en karim-en merged commit 12d3fee into near:master May 29, 2024
@mooori mooori mentioned this pull request Jun 3, 2024
This was referenced Mar 27, 2026
This was referenced Apr 7, 2026
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.

5 participants