Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Handful of fixes, mostly related to project compilation. #267

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

Closed
LennardF1989 wants to merge 6 commits into lowcoder-org:main from LennardF1989:dev/some-fixes

Conversation

Copy link

@LennardF1989 LennardF1989 commented Jul 6, 2023

This branch contains a handful of fixes, you can cherry pick whichever you like:

  • Fixed broken unit tests (or disabled them), this includes missing configuration.
  • Fixes some pom.xml stuff to make sure "mvn clean package" can actually finish.
  • Added a bunch of node-service fixes extracted from the OpenBlocks EE Docker image that they never published to their GitHub.

Copy link
Contributor

@FalkWolsky FalkWolsky 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 very much.
Generally we have to look about the UT-TACO-TOKEN.
In terms of concept and naming

Copy link
Contributor

@FalkWolsky FalkWolsky 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 very much!

Copy link
Contributor

@FalkWolsky FalkWolsky left a comment

Choose a reason for hiding this comment

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

Very nice. Thank you!

Copy link
Contributor

@FalkWolsky FalkWolsky left a comment

Choose a reason for hiding this comment

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

All changes are very good. Thank you

@LennardF1989 LennardF1989 changed the title (削除) Dev/some fixes (削除ここまで) (追記) Handful of fixes, mostly related to project compilation. (追記ここまで) Jul 6, 2023

@Test
public void testGoogleRegisterSuccess() {
assertTrue(true);
Copy link
Collaborator

@ludomikula ludomikula Jul 6, 2023

Choose a reason for hiding this comment

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

Generally I would not use a test like this because it gives false hope that the functionality works.
Better approach is to keep the test as is and annotate it with @disabled with a brief description:

@Disabled("Disabled until properly refactored")


@SuppressWarnings("ConstantConditions")
@Test
public void testUpdateApplicationFailedDueToLackOfDatasourcePermissions() {
Copy link
Collaborator

@ludomikula ludomikula Jul 6, 2023

Choose a reason for hiding this comment

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

It's better to keep the original test and annotate it with @disabled annotation with a brief description. Otherwise it gives false hope that the functionality works.

@Disabled("Disabled until properly refactored")

Copy link
Contributor

Thank you very much for your contribution! We took most of your suggestions and realized it in other branches in the meantime. Happily, we can close now this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ludomikula ludomikula ludomikula left review comments

@FalkWolsky FalkWolsky FalkWolsky approved these changes

Labels
None yet
Projects
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /