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

Switch-to-dev script should add packages instead of linking those #1531

Closed
pomek opened this issue Feb 14, 2019 · 7 comments
Closed

Switch-to-dev script should add packages instead of linking those #1531

pomek opened this issue Feb 14, 2019 · 7 comments
Assignees
Labels
package:dev squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".

Comments

@pomek
Copy link
Member

pomek commented Feb 14, 2019

We shouldn't link packages because during yarn install, all these packages will be removed (and installed from NPM).

I had some issues with that during testing the new test environment on CI. See: ckeditor/ckeditor5-paragraph@c118504. After switching to yarn add file:, all issues disappeared.

Following command should be called instead:

yarn add file:../ckeditor5-dev/packages/ckeditor5-dev-docs \
  file:../ckeditor5-dev/packages/ckeditor5-dev-tests \
  file:../ckeditor5-dev/packages/ckeditor5-dev-webpack-plugin \ 
  file:../ckeditor5-dev/packages/ckeditor5-dev-env \
  file:../ckeditor5-dev/packages/ckeditor5-dev-utils \
  file:../ckeditor5-dev/packages/jsdoc-plugins \
  -W

Unfortunately, this solution comes with other bug: all files are copied. It means, if you changed anything in the dev package, you need to install it once again.

@pomek pomek added the type:task This issue reports a chore (non-production change) and other types of "todos". label Feb 14, 2019
@ma2ciek
Copy link
Contributor

ma2ciek commented Feb 15, 2019

Maybe we could use yarn link?

@Reinmar Reinmar added this to the backlog milestone Mar 18, 2019
@pomek pomek removed this from the backlog milestone Feb 21, 2022
@pomek pomek added squad:devops Issue to be handled by the Devops team. package:dev labels Oct 20, 2022
@pomek
Copy link
Member Author

pomek commented Oct 26, 2022

The main goal is to use changes from ckeditor5-dev in the CKEditor 5 project.

Proposals:

(1) Use yarn workspaces

  • CKE 5 package.json - add a workspace to ../ckeditor5-dev/packages/*
  • (Perhaps we need to remove all node_modules/ directories from CKE 5 Dev).
  • yarn install
  • Revert changes in package.json in CKE 5.

(2) Replace versioned packages in package.json with link/portal

  • Iterate over all CKEditor 5 packages (including CF & Internal) and replace versions for ckeditor5-dev-* packages with a protocol (link or portal).
  • (Perhaps we need to remove all node_modules/ directories from CKE 5 Dev).
  • Update packages that exist in the ckeditor5-dev repository.
  • yarn install
  • Revert all changes.

@CKEditorBot CKEditorBot added the status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. label Nov 3, 2022
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Nov 15, 2022
@przemyslaw-zan
Copy link
Member

przemyslaw-zan commented Nov 16, 2022

Using yarn workspaces to link the repository located in parent directory of the workspace root is not supported by yarn:

In order to use workspaces we could use mrgit to synchronise the ckeditor5-dev repository inside the external directory. I've tested this approach, and most things work out of the box. There are some problems with automatic tests being launched from the root of cke5:

image

This can be circumvented by using -f option and skipping tests from ckeditor5-dev. Launching ckeditor5-dev tests from its own nested repository works as expected. This issue could probably be ironed out.

Another problem with this approach is that during the testing phase, we want to use ckeditor5-dev version available on npm, not the one from the master branch. This would require manual removal of that repository during each testing phase, both from the directory, as well as the mrgit.json file, before proceeding with the usual testing phase preparations.

Second option, Replace versioned packages in package.json with link/portal would require from each user one time setup, where they would define paths to the repositories which they want to link. Then, after calling switch-to-dev script, all of the packages from the listed repositories would be used as dependencies wherever possible. Then dependencies are updated, and the packages are linked.

One disadvantage to this approach is the fact that calling yarn install again would break the links. This can be circumvented by simply calling switch-to-dev script instead of yarn install, as it installs the dependencies as well.

@pomek
Copy link
Member Author

pomek commented Nov 20, 2022

It turned out we should fix #12887 before resolving the issue.

@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Nov 21, 2022
@pomek
Copy link
Member Author

pomek commented Nov 23, 2022

Recent highlights/updates regarding the issue:

  • As the first option ((1) Use yarn workspaces) couldn't be used, we tried to implement (2).
  • While implementing PoC, we found a blocker: The serveTranslations() function throws when using the linked package #12887.
  • After resolving it (with some troubles), the same error (The 'compilation' argument must be an instance of Compilation) comes directly from webpack sources.
  • We decided to implement the first proposed solution, but instead of leaving the root directory of CKEditor 5, we aim to clone ckeditor5-dev directly in the external/ directory.
  • Manual checks seem to work.
  • Now, we need to improve the workflow.
    • Support for checking out tags in mrgit (Support for tags cksource/mrgit#148) – it is needed to check out a repository to a tag when the testing phase starts.
    • Rewrite the development environment guide to introduce mrgit again.
    • Remove the existing mentions and usages of the switch-to-dev script.

@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. and removed status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. labels Nov 25, 2022
@CKEditorBot CKEditorBot added status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Dec 7, 2022
pomek added a commit that referenced this issue Dec 7, 2022
Docs: Added mrgit chapter. See #1531.

Other: Removed switch-to-dev-dev script. See #1531.
@pomek
Copy link
Member Author

pomek commented Dec 7, 2022

To sum up what we decided:

  • We decided to clone CKEditor 5-related repositories (ckeditor5-dev, linters, etc.) into the external/ directory.
  • To switch between tagged and master commits easily, we decided to improve mrgit – https://github.com/cksource/mrgit/releases/tag/v2.0.0.
  • The switch-to-dev-dev.sh script is no longer needed.
    • You can use mrgit instead.

pomek added a commit that referenced this issue Dec 7, 2022
Docs: Added mrgit chapter. See #1531.

Other: Removed switch-to-dev-dev script. See #1531.
@pomek
Copy link
Member Author

pomek commented Dec 7, 2022

Covered by #12970.

@pomek pomek closed this as completed Dec 7, 2022
@pomek pomek added this to the iteration 60 milestone Dec 7, 2022
@pomek pomek modified the milestones: iteration 60, iteration 59 Dec 7, 2022
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:dev squad:devops Issue to be handled by the Devops team. type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants