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

NO-ISSUE: Make examples better reflect their domain and contain more focused code #2726

Merged
merged 37 commits into from
Dec 11, 2024

Conversation

tiagobento
Copy link
Contributor

@tiagobento tiagobento commented Nov 5, 2024

In this PR I'm improving almost all examples to have a more descriptive README and package name. I also split the huge webapp example to make our builds faster, separating the serverless-workflow-editor-on-webapp example be built as part of partition1.txt.

In this PR:

  • Split examples/webapp into several different examples, to make our builds faster and to have the examples have less boilerplate code, allowing users to focus on the content that matters for each individual component usage inside webapps:
    • examples/bpmn-editor-classic-on-webapp
    • examples/dmn-editor-classic-on-webapp
    • examples/dmn-editor-on-webapp
    • examples/dmn-editor-standalone-on-webapp
    • examples/micro-frontends-multiplying-architecture-base64png-editor-on-webapp
    • examples/micro-frontends-multiplying-architecture-ping-pong-views-on-webapp
    • examples/micro-frontends-multiplying-architecture-todo-list-view-on-webapp
    • examples/serverless-workflow-editor-standalone-on-webapp
  • Renamed examples/commit-message-validation-service to examples/kie-sandbox-commit-message-validation-service
  • Renamed examples/jbpm-compact-architecture-example to examples/process-compact-architecture
  • Renamed example/drools-process-usertasks-quarkus-example to examples/process-user-tasks-subsystem
  • Prefixed examples/ping-pong* and examples/todo-list* packages with micro-frontends-multiplying-architecture, to make it clear that those examples are related to Multiplying Architecture. The "Micro-frontends" part is to not have them be confused with Compact Architecture, which is a completely unrelated term used to signify the embedding of Data-Index, Jobs Service, and Data-Audit on Process applications.
  • Revisited the READMEs of all examples
  • Added a very short README to the packages and examples folder.

I know this is quite a lot, but there's not much difference in content. I recommend checking out this branch and looking at each of the changed examples individually, they're all quite small and simple, so shouldn't be a big effort to do it.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

kie-tools-examples-micro-frontends-multiplying-architecture-base64png-editor-vscode-extension

is not consistent in naming:

  • kie-tools-examples-micro-frontends-multiplying-architecture-base64png-editor-vscode-extension
  • vs @kie-tools-examples/micro-frontends-multiplying-architecture-base64png-editor-vscode-extension,

this is also incompatible with the build command [1] KIE_TOOLS_BUILD__buildExamples=true pnpm -F @kie-tools-examples/micro-frontends-multiplying-architecture-base64png-editor-vscode-extension^... build:dev

Probably also build:prod command should contain -F part.

🔴 build command [1] failed

│ > copyfiles -u 1 "src/**/*.{sass,scss,css}" dist/
└─ Running...
packages/vscode-extension build:dev$ rimraf dist && tsc -p tsconfig.json
│ src/workspace/VsCodeResourceContentServiceForWorkspaces.ts(33,27): error TS2307: Cannot find module 'isomorphic-git' or its corresponding type declarations.
│ src/workspace/VsCodeResourceContentServiceForWorkspaces.ts(81,10): error TS7006: Parameter 'p' implicitly has an 'any' type.
└─ Failed in 9.5s at /home/jomarko/kie-tools/packages/vscode-extension
/home/jomarko/kie-tools/packages/vscode-extension:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @kie-tools-core/[email protected] build:dev: `rimraf dist && tsc -p tsconfig.json`
Exit status 2

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view

No manual checks done, README looks fine.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view-in-angular

No manual checks done. From README it is not clear to me how to start the example.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view-in-react

same as above ^

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-views-on-webapp

Start command in readme could include -F part.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-todo-list-view

No checks done, no comments to code.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/micro-frontends-multiplying-architecture-todo-list-view-on-webapp

Readme start command could include -F part probably

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

kie-tools-examples-micro-frontends-multuplying-architecture-todo-list-view-vscode-extension

Similar issues as base64png vscode extension package

│ [run-script-if] Finished 'pnpm build'
└─ Done in 9s
packages/editor build:dev$ rimraf dist && pnpm copy:css && tsc -p tsconfig.json
│ > @kie-tools-core/[email protected] copy:css /home/jomarko/kie-tools/packages/editor
│ > copyfiles -u 1 "src/**/*.{sass,scss,css}" dist/
└─ Done in 9.2s
packages/vscode-extension build:dev$ rimraf dist && tsc -p tsconfig.json
│ src/workspace/VsCodeResourceContentServiceForWorkspaces.ts(33,27): error TS2307: Cannot find module 'isomorphic-git' or its corresponding type declarations.
│ src/workspace/VsCodeResourceContentServiceForWorkspaces.ts(81,10): error TS7006: Parameter 'p' implicitly has an 'any' type.
└─ Failed in 6.1s at /home/jomarko/kie-tools/packages/vscode-extension
/home/jomarko/kie-tools/packages/vscode-extension:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @kie-tools-core/[email protected] build:dev: `rimraf dist && tsc -p tsconfig.json`
Exit status 2

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

[edited]

@kie-tools-examples/process-compact-architecture

🟠 I am not able to use startContainers script and follow readme steps, until I build the example with KOGITO_RUNTIME_version as 999-SNAPSHOT, otherwise user will get the error below when starting a process instance.

nator.beforeCompletion - failed for SynchronizationImple< 0:ffffac120005:a927:6735b50d:190, org.hibernate.resource.transaction.backend.jta.internal.synchronization.RegisteredSynchronization@1fcaee53 >: org.hibernate.exception.SQLGrammarException: could not execute statement [ERROR: column "recipient" is of type jsonb but expression is of type bytea
process-compact-architecture-service  |   Hint: You will need to rewrite or cast the expression.
process-compact-architecture-service  |   Position: 221] [insert into job_details (correlation_id,created,execution_counter,execution_timeout,execution_timeout_unit,fire_time,last_update,priority,recipient,retries,scheduled_id,status,trigger,id) values (?,?,?,?,?,?,?,?,?,?,?,?,?,?)]
process-compact-architecture-service  |         at org.hibernate.exception.internal.SQLStateConversionDelegate.convert(SQLStateConversionDelegate.java:91)
process-compact-architecture-service  |         at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:58)
process-compact-architecture-service  |         at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:108)
process-compact-architecture-service  |         at org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.executeUpdate(ResultSetReturnImpl.java:197)
process-compact-architecture-service  |         at org.hibernate.engine.jdbc.mutation.internal.AbstractMutationExecutor.performNonBatchedMutation(AbstractMutationExecutor.java:108)
process-compact-architecture-service  |         at org.hibernate.engine.jdbc.mutation.internal.MutationExecutorSingleNonBatched.performNonBatchedOperations(MutationExecutorSingleNonBatched.java:40)
process-compact-architecture-service  |         at org.hibernate.engine.jdbc.mutation.internal.AbstractMutationExecutor.execute(AbstractMutationExecutor.java:52)
process-compact-architecture-service  |         at org.hibernate.persister.entity.mutation.InsertCoordinator.doStaticInserts(InsertCoordinator.java:175)
process-compact-architecture-service  |         at org.hibernate.persister.entity.mutation.InsertCoordinator.coordinateInsert(InsertCoordinator.java:113)
process-compact-architecture-service  |         at org.hibernate.persister.entity.AbstractEntityPersister.insert(AbstractEntityPersister.java:2863)
process-compact-architecture-service  |         at org.hibernate.action.internal.EntityInsertAction.execute(EntityInsertAction.java:104)
process-compact-architecture-service  |         at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:632)
process-compact-architecture-service  |         at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:499)
process-compact-architecture-service  |         at org.hibernate.event.internal.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:363)
process-compact-architecture-service  |         at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:41)```

We should mention `docker compose down` as prerequisite, I think.

**mvn clean package quarkus:dev** seems to work fine 🟢 

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/process-user-tasks-subsystem

🔴 there is a compilation error

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/jomarko/kie-tools/examples/process-user-tasks-subsystem/src/test/java/org/acme/travels/quarkus/ApprovalsProcessTest.java:[28,48] package org.jbpm.process.instance.impl.humantask does not exist
[ERROR] /home/jomarko/kie-tools/examples/process-user-tasks-subsystem/src/test/java/org/acme/travels/quarkus/ApprovalsProcessTest.java:[29,55] package org.jbpm.process.instance.impl.humantask.phases does not exist
[ERROR] /home/jomarko/kie-tools/examples/process-user-tasks-subsystem/src/test/java/org/acme/travels/quarkus/ApprovalsProcessTest.java:[30,47] package org.jbpm.process.instance.impl.workitem does not exist

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/serverless-workflow-editor-standalone-on-webapp

Manual check passed, start command could maybe include -F part.

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/sonataflow-greeting

No comments

@jomarko
Copy link
Contributor

jomarko commented Nov 14, 2024

@kie-tools-examples/uniforms-patternfly

Readme could contain at least information how to build and start the example.

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

Feedback shared via one comment per package. Thank you @tiagobento

@tiagobento
Copy link
Contributor Author

tiagobento commented Nov 20, 2024

General comments:

  • All commands in READMEs assume you're in the example's folder.
  • Bootstrapping the repo with pnpm bootstrap is always necessary.

@kie-tools-examples/dmn-editor-classic-on-webapp

✅ Fixed


@kie-tools-examples/dmn-editor-on-webapp

✅ Fixed


@kie-tools-examples/dmn-editor-on-webapp

✅ Fixed


@kie-tools-examples/dmn-editor-standalone-on-webapp

🟡 Updated the port. Will investigate the failures.


@kie-tools-examples/kie-sandbox-commit-message-validation-service

✅ There are no build instructions necessary here.. Simply "pnpm start" suffices.


@kie-tools-examples/micro-frontends-multiplying-architecture-base64png-editor

✅ True. This is not runnable. Mentioned in the README.


@kie-tools-examples/micro-frontends-multiplying-architecture-base64png-editor-chrome-extension

✅ There are some in this repo! :) Or you can use the Base64 Editor VS Code Extension to create some based on PNG files.


@kie-tools-examples/micro-frontends-multiplying-architecture-base64png-editor-on-webapp

✅ Nothing done.


kie-tools-examples-micro-frontends-multiplying-architecture-base64png-editor-vscode-extension

✅ Not having the scope (@kie-tools) part in the name of the package is a requirement from the VS Code Marketplace. Fixed the build command. Build failed for you because you didn't bootstrap first, I guess?


@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view

✅ OK


@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view-in-angular

✅ This is not runnable. I made it clear on the README now.


@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-view-in-react

✅ Same as above.


@kie-tools-examples/micro-frontends-multiplying-architecture-ping-pong-views-on-webapp

✅ OK


@kie-tools-examples/micro-frontends-multiplying-architecture-todo-list-view

🟡 Not sure I understand your feedback here...


@kie-tools-examples/micro-frontends-multiplying-architecture-todo-list-view-on-webapp

✅ OK


kie-tools-examples-micro-frontends-multuplying-architecture-todo-list-view-vscode-extension

✅ Please see the other VS Code extension example's comment from my end as they apply here too.


@kie-tools-examples/process-compact-architecture

✅ True... this is expected, and we need to upgrade the Kogito version of the repo. I'll do it soon in a separate PR.


@kie-tools-examples/process-user-tasks-subsystem

🟡 Weird... Will investigate. Thanks!


@kie-tools-examples/serverless-workflow-editor-standalone-on-webapp

✅ OK


@kie-tools-examples/sonataflow-greeting

✅ Out of scope of this PR.


@kie-tools-examples/uniforms-patternfly

✅ Out of scope of this PR.

@tiagobento
Copy link
Contributor Author

@kie-tools-examples/process-user-tasks-subsystem

✅ Removed the test... The example compiles normally now. We currently don't have tests for our examples, but I guess if we want that, we need a new "examples-tests" top-level directory to host them.


@kie-tools-examples/micro-frontends-multiplying-architecture-todo-list-view

✅ As we discussed, this is ok.


@kie-tools-examples/dmn-editor-standalone-on-webapp

✅ Took me a while to read the comment, but I finally did:

// The paths are important here! Since path2/loan-pre-qualification.dmn is in a
// different parent path than the main content (path1/can-drive.dmn), it won't be availabe
// to be used as an included model.

I added a comment to the specific line pointing to the main comment:

["path2/loan-pre-qualification.dmn", { contentType: "text", content: loadFile("loan-pre-qualification.dmn") }], // Won't be available! Read comment above.


Hope it's in good shape now @jomarko. Thanks!

.rat-excludes Outdated Show resolved Hide resolved
Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

thank you @tiagobento , two very minor comments before approval

@tiagobento
Copy link
Contributor Author

@jomarko Thanks! Comments addressed in bb960bd.

@ljmotta ljmotta merged commit c681c8c into apache:main Dec 11, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants