Open
Conversation
Collaborator
|
@Jules-Bertholet Have you seen ckeditor/ckeditor5-dev#594? Do you think it'd impact this? Bikeshed: Do you have any thoughts if that'd impact the need to include a submodule dependency? I'm interested in avoiding that if possible. I suppose another option could be to clone the ckeditor5 repo via a script and have it under |
Contributor
Author
|
No, ckeditor/ckeditor5-dev#594 as is wouldn't eliminate the need for getting files from the main repo (note that all that is needed is the contents of https://github.com/ckeditor/ckeditor5/tree/master/packages/ckeditor5-core/tests/_utils). |
Collaborator
|
@Jules-Bertholet Any comments on using a shell script that'd check if it exists and clone it rather than a submodule? The reason I ask is I think submodules are heavy. Development and building doesn't need them. If they're only part of tests, we may be able to avoid attaching the repo a the git level. Also we may be able to use commands that clone with less depth ( If you'd like I can propose something |
Contributor
Author
Go ahead |
Collaborator
|
@Jules-Bertholet Absolutely Note: I may not be able to get around to that immediately. I'm assuming the "allow contributions from maintainers" is checked in the right side of the PR though? |
Contributor
Author
|
It is checked, yes |
Collaborator
|
@Jules-Bertholet Could you rebase this PR? (to fix the README conflict?) I reformatted the README with prettier in fef1e16 Could you take a stab at this w/o the git submodule? e.g. creating a shell script + npm The reason why is git submodules are heavy duty for a one-off case and it breaks from the pattern of using npm packages. |
This commit introduces a Git submodule to the repo
Contributor
Author
|
I've updated the instructions for fetching the submodule so that only needed files are downoaded (this makes the submodule solution much lighter). |
Contributor
Author
|
@tony I've added a |
Contributor
Author
|
Note that once ckeditor/ckeditor5#7311 is merged we will be able to dump the submodule completely. |
Collaborator
|
One detail - I think we should avoid the git submodule part unless we've no other option. Do you use them often? I do but want to try without it for this case. The reason why is this is only used for tests and I think submodules would be for cases where we fundamentally relied on it (even for normal dev/packaging, but we use NPM for that). If it's true ckeditor/ckeditor5#7311 is near, that is something I'd like to explore since it fits the packaging setup here better. Have you considered |
Contributor
Author
|
I've eliminated the submodules |
Collaborator
|
Thank you!!! The
This was due to my git version: I updated to |
Collaborator
|
When running locally, here is what I get: |
Collaborator
|
@Jules-Bertholet Would you like to take a look at this and see if you get the same error when trying again? (e.g. If not, I can merge this in and try to fix after. My plan is to set up a GH action to run these |
Contributor
Author
|
Should work now |
Collaborator
|
@Jules-Bertholet I think this needs to pull the correct tag from ckeditor5 Do tests pass for you with Even with |
Contributor
Author
|
No, tests don't pass for me. I only intended to get them to run, never look at why they were failing |
tony
reviewed
Jun 21, 2021
| @@ -0,0 +1,2 @@ | |||
| { | |||
| } No newline at end of file | |||
Collaborator
There was a problem hiding this comment.
@Jules-Bertholet Is this needed? We could also at .vscode/, .vim/ to .gitignore
Collaborator
Roger that! That's still helpful! |
jamieconnelly
added a commit
to Multiverse-io/ckeditor5-math
that referenced
this pull request
Oct 21, 2024
Tests currently do not work see - isaul32#43 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #36. Unfortunately this required adding the main ckeditor5 repo as a submodule, you will have to run
git submdule initandgit submodule update.To actually run the tests, use
yarn test, optionally with--coverage,--watch, or--source-map.