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

loadSchemaDialects function #53

Merged
merged 3 commits into from
Mar 14, 2024
Merged

Conversation

unix-unicorn
Copy link

@unix-unicorn unix-unicorn commented Mar 7, 2024

Made a simple function to expose the _dialects object keys and filtering with hasDialect to make sure that we get the loaded schema dialects for json-schema LSP Project

Co-authored-by: Azzam Uddin uazzam69@gmail.com @Azzam1503

@unix-unicorn unix-unicorn changed the base branch from main to lsp March 7, 2024 20:18
Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Please include tests and documentation (in the README) for your new function.

lib/keywords.js Outdated Show resolved Hide resolved
lib/keywords.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lib/get-dialect-ids.spec.ts Outdated Show resolved Hide resolved
lib/keywords.js Outdated Show resolved Hide resolved
lib/get-dialect-ids.spec.ts Show resolved Hide resolved
lib/get-dialect-ids.spec.ts Outdated Show resolved Hide resolved
lib/get-dialect-ids.spec.ts Outdated Show resolved Hide resolved
@jdesrosiers
Copy link
Collaborator

This PR is a mess and can't be reviewed or merged in it's current state. This is why I told you to use rebase to update your branch instead of merge. You need to drop your merge commit and rebase properly.

@Azzam1503 Azzam1503 force-pushed the get-schemas branch 2 times, most recently from 994fbdf to c2f9e1d Compare March 13, 2024 13:53
Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Just a couple more details to clean up. We're almost there!

(It looks like some of the commits got squashed in the attempt to clean up after the merge, but it appears that all the content is there.)

lib/get-dialect-ids.spec.ts Outdated Show resolved Hide resolved
lib/get-dialect-ids.spec.ts Outdated Show resolved Hide resolved
lib/keywords.js Outdated Show resolved Hide resolved
@Azzam1503 Azzam1503 force-pushed the get-schemas branch 2 times, most recently from bcaddc4 to bdf76d7 Compare March 13, 2024 22:18
Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Looks good 🎉. I'll wait until to tomorrow to merge to give @HodaSalim a chance to fix the author meta-data on her commits.

unix-unicorn and others added 3 commits March 14, 2024 01:54
Made a simple function to expose the _dialects object keys and filtering with hasDialect to make sure that we get the loaded schema dialects for json-schema LSP Project
Co-authored-by: Azzam Uddin <uazzam69@gmail.com>
@unix-unicorn
Copy link
Author

Looks good 🎉. I'll wait until to tomorrow to merge to give @HodaSalim a chance to fix the author meta-data on her commits.

Thank you so much ! It should be updated now

@jdesrosiers jdesrosiers merged commit f1efa88 into hyperjump-io:lsp Mar 14, 2024
2 checks passed
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