8000 chore: refactor procedures namespace by BeksOmega · Pull Request #6745 · google/blockly · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: refactor procedures namespace #6745

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
Merged
merged 3 commits into from
Jan 11, 2023
Merged

chore: refactor procedures namespace #6745

merged 3 commits into from
Jan 11, 2023

Conversation

BeksOmega
Copy link
Collaborator

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes #6525

Proposed Changes

Refactors the procedures namespace to use the new procedure block interface methods instead of the old ones.

Should be no change in behavior.

Reason for Changes

Realized I actually did need to refactor this so that external devs don't have to implement the old getProcedureDef and getProcedureCall methods.

Test Coverage

Passes all existing tests.

Documentation

N/A

Additional Information

Dependent on #6736

@github-actions github-actions bot added the PR: chore General chores (dependencies, typos, etc) label Jan 9, 2023
@BeksOmega BeksOmega mentioned this pull request Jan 9, 2023
Merged
4 tasks
@BeksOmega BeksOmega force-pushed the chore/refactor-procedures-namespace branch from 3a1391d to 4301abf Compare January 10, 2023 00:23
@BeksOmega BeksOmega marked this pull request as ready for review January 10, 2023 01:58
@BeksOmega BeksOmega requested a review from a team as a code owner January 10, 2023 01:58
@BeksOmega BeksOmega requested a review from gonfunko January 10, 2023 01:58
gonfunko
gonfunko approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor
@gonfunko gonfunko left a comment

Choose a reason for hiding this comment

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

Do we want to mark this as a breaking change? I realize the root blocks are updated to accommodate it, but since the interface changed it might still be worth calling out?

@BeksOmega
Copy link
Collaborator Author

All the changes I made should be backwards compatible (that was my goal anyway). Do you see something I missed?

@gonfunko
Copy link
Contributor

All the changes I made should be backwards compatible (that was my goal anyway). Do you see something I missed?

Just the change to the IProcedureBlock interface; theoretically somebody could have created a class that implements it and doesn't inherit from the core procedure blocks, in which case it would break I think? But I can easily be convinced that's enough of an edge case to not worry about.

@BeksOmega
Copy link
Collaborator Author

All the changes I made should be backwards compatible (that was my goal anyway). Do you see something I missed?

Just the change to the IProcedureBlock interface; theoretically somebody could have created a class that implements it and doesn't inherit from the core procedure blocks, in which case it would break I think? But I can easily be convinced that's enough of an edge case to not worry about.

Ah sweet, that was only released in December, and according to our readme: "Once a new API is merged into master it is considered beta until the following release." so we get away with this one scot-free 😎

@BeksOmega BeksOmega merged commit 902c309 into google:develop Jan 11, 2023
@BeksOmega BeksOmega mentioned this pull request Jan 17, 2023
Closed
@BeksOmega BeksOmega deleted the chore/refactor-procedures-namespace branch May 3, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gonfunko gonfunko gonfunko approved these changes

Assignees

@gonfunko gonfunko

Labels
PR: chore General chores (dependencies, typos, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor the Procedures namespace
2 participants
@BeksOmega @gonfunko
0