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

Refactor command registration and tree expansion events #202

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

AndrewTwydell
Copy link
Contributor

What It Does
Refactor extension.ts to reduce repetition. The majority of the expansion events were 1 of 2 methods, with a couple of exceptions. The refactor ensures the common behaviour share a single method.

I also moved the commands registration out of the file to it's own method for tidiness.

Review Checklist
I certify that I have:

Copy link
Contributor

@davenice davenice left a comment

Choose a reason for hiding this comment

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

I think this looks sensible. I like beginning to split out extension.ts because it will reduce the number of merge conflicts we hit.

I like the map of types to actions.

I've tested this locally, expanding a full tree in a CICSplex and in an SMSS region.

Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

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

I really like how clean the extension.ts file looks after refactoring 🙏

LGTM! 😋

@zFernand0 zFernand0 merged commit 22639c6 into main Jan 15, 2025
17 checks passed
@zFernand0 zFernand0 deleted the extension-refactor branch January 15, 2025 22:03
@zFernand0 zFernand0 added the release-current Indicates that there is no new functionality being delivered label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog release-current Indicates that there is no new functionality being delivered
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

3 participants