Skip to content

refactor: componentize changes #203

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

vados-cosmonic
Copy link
Contributor

@vados-cosmonic vados-cosmonic commented Mar 19, 2025

This refactoring makes a few changes to the code to make componentize-js a bit easier to debug:

  • enable passing debug options all together
  • enable passing where to output debug output
  • split out function for easier reading

@vados-cosmonic
Copy link
Contributor Author

Setting this to draft while #202 lands -- the functionality in here will have to change quite a bit.

@vados-cosmonic vados-cosmonic marked this pull request as draft March 19, 2025 15:19
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch 5 times, most recently from 799820f to e8c8650 Compare March 21, 2025 08:33
@vados-cosmonic vados-cosmonic marked this pull request as ready for review March 21, 2025 08:34
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch 3 times, most recently from a373f76 to 391894e Compare March 21, 2025 10:04
Copy link
Member

@tschneidereit tschneidereit left a comment

Choose a reason for hiding this comment

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

Would you by any chance be able to split this into two commits, where one only does pure movement of code that's otherwise unmodified, and the other makes whatever functional or structural changes are contained?

I started reviewing this, but found myself jumping around and trying to follow where there are real changes, vs where a function has just moved to a different location in the file, and found that very hard to do.

@tschneidereit
Copy link
Member

(I appreciate that my last big PR to this repo did a lot of the same. I tried minimizing code movement in that, but in hindsight should nevertheless have split the movement that I did apply off into its own commit.)

@vados-cosmonic
Copy link
Contributor Author

Definitely can do -- I'll split this into more commits so it's a bit easier

@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch 2 times, most recently from 5924e1c to a98a681 Compare March 27, 2025 16:43
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch from a98a681 to 99c8460 Compare March 27, 2025 17:13
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch from 1550cdc to 99c8460 Compare March 27, 2025 17:33
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch 3 times, most recently from e144a6d to 5a5d5f3 Compare March 27, 2025 18:02
@vados-cosmonic vados-cosmonic force-pushed the refactor=componentize-changes branch from 5a5d5f3 to 52c2981 Compare March 27, 2025 18:08
@vados-cosmonic
Copy link
Contributor Author

Hey @tschneidereit so it turned into 8 commits 😅 -- hopefully these changes are easier to follow.

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.

2 participants