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

[Feature] Add <SEPARATOR> special word #296

Open
wants to merge 7 commits into
base: v6
Choose a base branch
from
Open

Conversation

joutvhu
Copy link

@joutvhu joutvhu commented Jun 1, 2024

Implement feature #268

@joutvhu
Copy link
Author

joutvhu commented Jun 1, 2024

Hi @ayusharma, @byara
Please help to review

@forivall
Copy link

forivall commented Dec 6, 2024

This will need my additional changes (joutvhu/prettier-plugin-sort-imports#2) before merging, as adding tests revealed some critical issues.

@byara byara changed the base branch from main to v5.2.0 December 8, 2024 14:58
@byara byara changed the base branch from v5.2.0 to main December 8, 2024 15:01
@byara
Copy link
Collaborator

byara commented Dec 8, 2024

Hey @joutvhu can we please update this PR with some tests mentioned by @forivall?

@byara byara changed the base branch from main to v6 December 9, 2024 10:49
@byara
Copy link
Collaborator

byara commented Dec 9, 2024

I changes the base branch to v6. Let's release it there.

@joutvhu
Copy link
Author

joutvhu commented Dec 9, 2024

@byara I added tests

joutvhu and others added 3 commits December 10, 2024 00:42
Add tests for the separator special word
# Conflicts:
#	src/utils/get-sorted-nodes.ts
Copy link

@forivall forivall Dec 12, 2024

Choose a reason for hiding this comment

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

this doesnt have any comments? I already included similar tests in ImportsManualSeparated, so i think these Separator tests can be removed or merged into that folder.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @joutvhu can you please address this comment?
There might be some other tests within a copy of this PR from here, it would be nice to add those too.

Copy link
Collaborator

@byara byara 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 for the PR 👏 really appreciate the effort you have put into this!
I left a comment, could you please take a look?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @joutvhu can you please address this comment?
There might be some other tests within a copy of this PR from here, it would be nice to add those too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants