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

Update Ace mode paths for tests #6002

Merged
merged 2 commits into from
Aug 13, 2022
Merged

Conversation

lildude
Copy link
Member

@lildude lildude commented Aug 8, 2022

The upstream directory structure was changed in ajaxorg/ace#4851 which means we're looking in the wrong place for all the languages supported by the Ace editor. We now need to look in the old location for jsoniq and xquery and the new location for everything else.

These changes have been made as they plan to release Ace as an NPM.

The directory structure was changed in ajaxorg/ace#4851
@Alhadis
Copy link
Collaborator

Alhadis commented Aug 8, 2022

There doesn't appear to be any rationale behind ajaxorg/ace#4851. If upstream makes this sort of change willy-nilly, then it might be wiser to query the latest tagged release, as opposed to HEAD. Doing so will reduce the risk of this happening again, since there's no guarantee such an arbitrary change won't be reverted later.

@Bamblehorse Bamblehorse mentioned this pull request Aug 9, 2022
1 task
@lildude
Copy link
Member Author

lildude commented Aug 10, 2022

There doesn't appear to be any rationale behind ajaxorg/ace#4851.

There is now as I asked for one 😁.

Based on this, I think we're good to stay with using HEAD. This has been the first major change like this since we implemented the test so I think we should be good querying both locations for the future.

@lildude lildude marked this pull request as ready for review August 10, 2022 10:30
@lildude lildude requested a review from a team as a code owner August 10, 2022 10:30
@lildude lildude changed the title Update ace mode path for tests Update Ace mode paths for tests Aug 11, 2022
@lildude lildude merged commit 1f65799 into master Aug 13, 2022
@lildude lildude deleted the lildude/update-ace-modes-test branch August 13, 2022 10:18
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants