Skip to content

Conversation

@Achal1607
Copy link
Member

@Achal1607 Achal1607 commented Oct 11, 2024

The main aim of refactoring was to make code more modular so that it is more readable, scalable and maintainable.

  • extension.ts is refactored into multiple modules
  • LSP server startup is simplified
  • commands and configuration modules are created
  • Debugger is separated out in a different module
  • Logger and other utils files are created.

NOTE: Views are yet to be refactored which can be done in a separate PR or included in this only once it is done.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Oct 11, 2024
@Achal1607 Achal1607 marked this pull request as ready for review October 11, 2024 14:44
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

This is a really useful and massive piece of work you have spearheaded @Achal1607. Thank you very much.
I'm still at the 1/3rd mark of completing the review but I thought I'll publish the 1 minor actionable comment till now.

@sid-srini sid-srini self-requested a review October 22, 2024 05:35
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

I've done one pass of glancing over the code changes and added some minor changes required and some doubts/questions. Thanks.

sid-srini
sid-srini previously approved these changes Oct 22, 2024
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

LGTM :)
Please git squash/fixup groups of commits that are related, so that merged history will be better organised.
Thanks a lot!

sid-srini
sid-srini previously approved these changes Oct 22, 2024
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

Thanks for organising the commits structure prior to merge. LGTM.

@naren2605
Copy link
Member

thanks for taking up this and refactoring @Achal1607 , its a big change. code looks more organised now

sid-srini
sid-srini previously approved these changes Oct 25, 2024
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

Looks fine. Thanks a lot.
Please go ahead and merge after completion of the preliminary test rounds. Thanks.

@Achal1607 Achal1607 force-pushed the lsp-refactored-code branch from 788bb46 to cffad1f Compare October 25, 2024 10:46
Copy link
Member

@sid-srini sid-srini left a comment

Choose a reason for hiding this comment

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

👍 Thank you. LGTM.

@Achal1607 Achal1607 merged commit 08ed2f7 into oracle:main Oct 26, 2024
3 checks passed
@sid-srini sid-srini removed the do not merge Don't merge PR label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants