Skip to content

Reorganize constants in packages/core#1987

Merged
jonahtanjz merged 1 commit into
MarkBind:masterfrom
tlylt:refactor-page
Aug 11, 2022
Merged

Reorganize constants in packages/core#1987
jonahtanjz merged 1 commit into
MarkBind:masterfrom
tlylt:refactor-page

Conversation

@tlylt

@tlylt tlylt commented Aug 3, 2022

Copy link
Copy Markdown
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
An initial attempt to refactor existing code within packages/core. To keep changes manageable and easy to review, this PR only addresses the following:

  • Relocate constants back to relevant files if they are not used in more than one place
  • Remove unused constants
  • Use newlines to visually group constants better
  • Minor typo fix
  • Remove outdated comments
  • Use defined constants if available
  • Extract HTML string to constant

Anything you'd like to highlight/discuss:
Previously, an attempt was made to separate constants into individual files in #949. While there is nothing wrong with the approach, I feel that re-evaluating the existing constants and shifting them into appropriate locations could help reduce unnecessary exports and set the stage for further refactoring. There could be personal preference involved so up for discussion on whether this PR improves the codebase.

Testing instructions:
nil, this refactor should not affect existing functionalities and hence CI checks should be sufficient

Proposed commit message: (wrap lines at 72 characters)
Reorganize constants in packages/core


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@tlylt tlylt requested a review from ang-zeyu August 3, 2022 07:36

@ang-zeyu ang-zeyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel that re-evaluating the existing constants and shifting them into appropriate locations could help reduce unnecessary exports and set the stage for further refactoring. There could be personal preference involved so up for discussion on whether this PR improves the codebase.

Agreed here, this reorganization looks good! 👍

@jonahtanjz jonahtanjz added this to the 4.0.1 milestone Aug 10, 2022
@jonahtanjz jonahtanjz merged commit c87d394 into MarkBind:master Aug 11, 2022
@ang-zeyu ang-zeyu mentioned this pull request Aug 22, 2022
11 tasks
ang-zeyu added a commit that referenced this pull request Sep 3, 2022
A similar PR is #1942 (closed), but it was mostly made
redundant with #1987 which reorganized all constants.
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.

3 participants