zeebe-io/zeebe-modeler

Pull in Camunda Modeler v4.2 changes

pinussilvestrus opened this issue · 8 comments

pinussilvestrus commented

What should we do?

Pull in camunda-modeler@4.2.0 changes (once it got released).

$ npm run sync -- -t 'v4.2.0'

Why should we do it?

We should be up to date and it also fixes a bug (cf. #69)

Child of #232

barmac commented

For some reason it does not pull in even bpmn-js update. Do you know what could be the reason?

▶ npm run sync -- -t 'v4.2.0'

> zeebe-modeler-builder@0.5.0 sync /Users/maciej/workspace/bpmn-io/zeebe-modeler
> node tasks/sync-fork.js "-t" "v4.2.0"

##### Started syncing #####
Sync: Execute 'git fetch camunda master --tags' .
Sync: Fetched actual state of upstream 'camunda'.
Sync: Execute 'git merge --no-commit --no-ff true'.
merge: true - not something we can merge

Sync: No changes to be adopted. Stopped syncing!
Sync: Deleted all non-related tags.
##### Stopped syncing #####
pinussilvestrus commented

It seems like its not fetching the tags from the Camunda Modeler upstream correctly. So what git fetch camunda master --tags is supposed to do. But I have no idea why it behaves like this.

barmac commented

All right. I will investigate that later.

pinussilvestrus commented

Maybe you can simply use the master branch directly, which is the default when executing npm run sync. I did a dry-run and it seems to work better. If you stumble over what has to be fixed inside the sync-script, feel free to create an issue or approach me ☎️ . We didn't use it a while ago.

We have to be careful with the App icon changes in resources/platform/**/*.png and build/*.png which may be ignored in the script: https://github.com/zeebe-io/zeebe-modeler/blob/master/tasks/sync-fork.js#L376

barmac commented

I believe the above is a bug in mri@1.1.5. When I run the sync script with mri@1.1.4, I ended up with:

▶ npm run sync -- -t v4.2.0

> zeebe-modeler-builder@0.5.0 sync /Users/maciej/workspace/bpmn-io/zeebe-modeler
> node tasks/sync-fork.js "-t" "v4.2.0"

##### Started syncing #####
Sync: Execute 'git fetch camunda master --tags' .
Sync: Fetched actual state of upstream 'camunda'.
Sync: Execute 'git merge --no-commit --no-ff v4.2.0'.
warning: Cannot merge binary files: resources/platform/linux/support/icon_48.png (HEAD vs. v4.2.0)
warning: Cannot merge binary files: resources/platform/linux/support/icon_16.png (HEAD vs. v4.2.0)
warning: Cannot merge binary files: resources/platform/linux/support/icon_128.png (HEAD vs. v4.2.0)
warning: Cannot merge binary files: docs/screenshot.png (HEAD vs. v4.2.0)
warning: Cannot merge binary files: build/icon.png (HEAD vs. v4.2.0)
warning: Cannot merge binary files: build/icon.ico (HEAD vs. v4.2.0)
warning: Cannot merge binary files: build/icon.icns (HEAD vs. v4.2.0)
warning: Cannot merge binary files: app/resources/favicon.png (HEAD vs. v4.2.0)

Sync: Syncing was not successful! There might be some mergeconflicts which have to be fixed before.
Sync: Execute 'git reset HEAD -- client/src/app/tabs/dmn/ client/src/app/tabs/cmmn/ client/test/mocks/cmmn-js/ client/test/mocks/dmn-js/ client/src/app/tabs/bpmn/modeler/features/apply-default-templates/ client/src/app/tabs/bpmn/util/**/*namespace* client/src/plugins/'.
Sync: Execute 'git clean -fd'.
(node:73202) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'includes' of null
    at /Users/maciej/workspace/bpmn-io/zeebe-modeler/tasks/sync-fork.js:297:25
    at Array.filter (<anonymous>)
    at /Users/maciej/workspace/bpmn-io/zeebe-modeler/tasks/sync-fork.js:296:45
    at Array.filter (<anonymous>)
    at /Users/maciej/workspace/bpmn-io/zeebe-modeler/tasks/sync-fork.js:293:43
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:73202) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:73202) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

...which seems to be OK as I need to just resolve merge conflicts. I'll get back to you if I have any problems with that.

  • 👍1
barmac commented

OK so this function fails if a conflict has not file:

MergeConflict {reason: "file location", file: null}
barmac commented

Do we want to pull in:

  • Sentry integration? (IMO: yes)
  • build on demand integration? (IMO: not yet)
  • ET integration? (not yet?)
  • update check? (IMO: not yet)
pinussilvestrus commented

I don't think we should include any plugin at all but handle them individually in separate tickets.

  • 👍1