Build maven archetypes for taskana adapter example projects

Currently in the TaskanaAdapter Repository there are three example submodules for the reason of documentation. For a potential new customer it’s probably a good idea to give the new customer the chance to generate one of such adapter with the taskana specific files with an taskana maven archetype. One of the examples (taskana-adapter-camunda-listener-example) is already generated with a camunda maven archetype and then afterwards enriched with taskana specific files.

Sascha Frevel

Mustapha Zorgati Comment: ← this has potential and can change how we handle our example projects

Efficiently jumping to tickets

I have configured custom search engines (for Google Chrome) in order to jump really quick to some tickets. I can show how I use the custom search engines to access tickets really fast and share my configuration.

Mustapha Zorgati

Sofie Hofmann (Unlicensed)

Easier / quicker reviews

Github has a suggestion feature for pull requests. This allows the reveiwer to write a code suggestion and allows the submitter to apply those changes in 1 click.

This can safe us a lot of time during our review process. - Let me show you how this works.

Mustapha Zorgati

DBWriter in history module

The DBWriter in the taskana-simplehistory-provider module currently sets up test data for the HISTORY_EVENTS table (and ONLY for this table). There is a lot of duplicated code from SampleDataGenerator of the taskana-data module.

Also the DBWriter seems faulty. Should we add a dependency to taskana-data in test scope and use the SampleDataGenerator instead?

joerg.heffner

We should keep a close eye on the possibility of a hard dependency to the history module from core.

The proposed solution will be used.

JaasExtension and WithAccessId/s

The JaasExtension class and the WithAccessId/s interfaces are located in the test package of taskana-core. Therefore they can’t be used in other modules. (Even if you have a dependency to core)

This makes it hard to test things e.g. in the history module. Currently you have to test without security.

Should we keep testing with an unsecured engine in these cases? This might not always be possible.

Should we copy/paste JaasExtension class and the interfaces?

Should we create a separate module which will contain them which then can be added as a dependency in test scope where it’s needed?

joerg.heffner

https://taskana.atlassian.net/browse/TSK-1277

DB Versions

Should we upgrade our database versions? Postgress, DB2, H2?

Mustapha Zorgati

Postgress: 10.X => TSK-1197
DB2: 11.1.4.4

HTTP Status Code return type

All our *NotFoundExceptions return a status code 404 (NOT FOUND). The DomainNotFoundException returns a status code 400 (BAD REQUEST).

Why do we have this special behaviour for the Domain?

Comment from Holger Hagen (Unlicensed) -

my understanding is, that 404 only applies to the resource identified by the URI. If we post a new resource which depends on something which does not exist, a 400 return code is fine.

See https://restfulapi.net/http-status-codes/ -

“400 […] Errors can be like malformed request syntax, invalid request message parameters, or deceptive request routing etc.”

“The 404 error status code indicates that the REST API can’t map the client’s URI to a resource but may be available in the future. “

Mustapha Zorgati

See Guidelines → Backend → Http status code return type

Creation of “How to develop”

We have a lot of findings from this meetings. Should we start document our decisions somewhere and add it with our development rules?

Mustapha Zorgati

=> Guidelines

Proper usage of assertj

Currently we have a lot of tests which are using asserts in a very complicated way and thus result in harder readable tests.

Let me show you some examples and their improvements.

Mustapha Zorgati

We always use assertThat(<test object>) only. Never have a function call within the parenthesis.

See Guidelines

assertJ Migration

I just found this script: https://github.com/joel-costigliola/assertj-core/blob/master/src/main/scripts/convert-junit5-assertions-to-assertj.sh
This will allow us to automatically migrate from Junit5 → assertJ without little effort.

Thoughts?

Precondition: we have to eliminate hamcrest first.

Mustapha Zorgati

https://taskana.atlassian.net/browse/TSK-1180

Disabled Tests

Currently we have 10 disabled Tests. How should we deal with them? Remove or invesitgate?

Update: Not 10 anymore …

Mustapha Zorgati

https://taskana.atlassian.net/browse/TSK-1182

Naming convention for tests

Currently different developers used different naming conventions for tests.

For example the Should_ExpectedBehavior_When_StateUnderTest convention (see TaskQueryTestImpl) or the test[Feature being tested] convention (see TaskAttachmentTest) or in the adapter with a lot of _ between words

Should we define a standard convention for everyone to stick to? If yes, does this apply only for new tests or should old test be "migrated"?

joerg.heffner

Test pattern for new tests:

should_ExpectedBehavior_When_StateUnderTest

Dedicated taskana travis github user

Currently we have a dedicated taskana github travis user (https://github.com/taskanaTravis ) which is supposedly used to write back the poms with the new versions to the taskana main repository after a relese. Unfortunately this has never really worked, because of reasons and nobody really cared. (see https://github.com/Taskana/taskana/commit/c90d1f903bc50776be7f4210fa676a46d31227fc for example)

Can we just delete that account and just use a github access token from Holger Hagen (Unlicensed) to allow travis to push commits in Holgers name.

This is done in about 5 Minutes (literally changing a travis environment variable..)

Mustapha Zorgati

We deleted the dedicated github user and used Mustapha Zorgati github credentials for travis

SonarQube

We finally got SonarQube set-up. Unfortunately currently it cannot run for PRs.

Lets discuss how we want to integrate SonarQube in our current workflow

Mustapha Zorgati

Everyone has to run his own sonarcloud instance and put a branch analysis link into their PR. See SonarCloud Integration

Definition of Reviews

Discussion of Review Process:

• What & What not to review.

• What is the goal? Knowledge Sharing, Bug Prevention? Catch unapproved changes? Solving possible Problems before merging?

• Who reviews? One or many or everyone? Only People working on similar tasks?

• How to comment on PR?

• When to approve a review? Fulfilling a checklist? Solving all comments?

• Is a Review needed if the PR was done by pair programming?

• How can we minimize the waiting time for PR? Idea: Do PR always first thing of the day, then continue working on tasks.

If you want a deep meaningful discussion please view this awsome talk first:
https://www.youtube.com/watch?v=jXi8h44cbQA

Benjamin Eckstein (Unlicensed)

Dedicated meeting for this on 24.01.2020.
The results are in our guidelines.

Semantic versioning in web frontend

Currently we use semantic versioning in the web frontend (^ and ~ in front of version). Because of this and the package-lock.json file sometimes the build breakes due to version mismatch with build and the package-lock file.

How should we deal with this?

Mustapha Zorgati

Remove semantic versioning and stick with static versions for now (until our test coverage in the frontend is good enough and we’re confident that it’ll catch errors from version updates)

https://taskana.atlassian.net/browse/TSK-1036

Using multiple Assertions framework

Currently we are using both Junit5 Assertions and hemcrest assertions.

Personally I think that using multiple framework for assertions in tests is not “clean”. We should discuss which one we should use in the future. Also Consider AssertJ
https://www.blazemeter.com/blog/hamcrest-vs-assertj-assertion-frameworks-which-one-should-you-choose/

Mustapha Zorgati

https://taskana.atlassian.net/browse/TSK-1025

Modified timestamp

Currently we are using the modified timestamp in our entities to determine whether an entity has been modified by a different user. If the requests are processed in the same millisecond this method will not catch anymore. Thus we override the previous changed entity.

How serious is this? Is it worth checking out Etags?

Mustapha Zorgati

Spike it. How much effort? Is it worth it?
https://taskana.atlassian.net/browse/TSK-1026

Testing session before releases for the UI

Should the whole team take some time to test the UI and all functionality before releases?

joerg.heffner

Outcome: explorative testing appointment within each release. but don’t forget to test frontend after merging.

Fix race conditions in tests

How should we deal with race conditions in integration tests?

Example: We have a test class that is deleting objects in the DB on some methods and counting a result in another method. The result of the list count depends on the execution order.

Idea: Split reading and modifying DB Tests. Reset DB @beforeeach for modifying test classes and @beforeall on reading.

Mustapha Zorgati

Create a ticket and implement the test splitting of reading and modifying test cases.

https://taskana.atlassian.net/browse/TSK-1027

Rename Module spring-base

The Module’s purpose is to serve as a login (stub) service. Naming it “base” is confusing

Benjamin Eckstein (Unlicensed)

https://taskana.atlassian.net/browse/TSK-949
Modules will be renamed.

Dependencies

We should find a way to update our dependencies. How should we tackle this issue?

Mustapha Zorgati

Outcome: Test for known security vulnerabilities and let the build fail if found.
Take care of incompatibilities with the customer. What about different spring boot versions?

dedicated test modules.

Currently, our rest and history applications are split very “uncommonly”. Why do we have this split and is it useful?

Mustapha Zorgati

Outcome: Ask Holger for the Reason. Maybe we can merge rest-spring-test with rest-spring.

Problematic dependency between TaskanaAdapter and TASKANA

Changes in TASKANA can easily break the adapter. This recently became especially obvious while refactoring TASKANA. How do we want to handle this in the future?

Refactoring of classes (changing imports) within Taskana breaks the next build of the adapter.

O1: The snapshot version of the adapter could use a fixed version of taskana
O2: Combine Adapter and Taskana into 1 Project, run maven / Travis to test both
O3: Do nothing, refactoring or breaking changes do not happen that often. Manual fixing if we notice a broken build.

Mustapha Zorgati & joerg.heffner

Voting Results:

O1: Benjamin Eckstein (Unlicensed) Holger Hagen (Unlicensed)
O2:

O3: Bernd Breier (Unlicensed) Holger Hagen (Unlicensed) joerg.heffner

The result is Do Nothing.

Length of Data Types

With the adaptation of the new HATE terminology the length of our data types has exceeded our maximum line length.

This currently forces us to ignore the checkstyle warnings in these cases and needs a better solution.

Christopher Heiting (Unlicensed)

No action required since the formatter solves the problem automatically.

Commit Messages

Sometimes I find some commit messages with this format “Comments from X”.

These do not match our current standard for commit messages.

Mustapha Zorgati

See Guidelines. The reviewer of the PR has to check the commit messages.

Use region to logically group and fold code

Currently we have a lot of huge classes which contain many methods.

In order to achieve more structure and readability we can use code folding. What do you guys think about that? Look at JaasExtension for example.

Question: what about or Eclipse colleagues?

Mustapha Zorgati

We will use code folding in case it seems to be appropriate.

SF: have a look on this page Setup Developer Environment for the plugin description.

Removal of pro.taskana.common.api.LoggerUtils

What’s the reason for this class? All methods within that class can be replaced with the standard toString implementation for that entity. See the list below for details

• listToString + setToString => https://docs.oracle.com/javase/8/docs/api/java/util/AbstractCollection.html#toString--

• mapToString => https://docs.oracle.com/javase/8/docs/api/java/util/AbstractMap.html#toString--

Mustapha Zorgati

The class LoggerUtils will be removed.

Sascha Frevel will create a ticket.

https://taskana.atlassian.net/browse/TSK-1247

Version updates of dependencies also included in Spring Boot

Is it a good idea to update every dependency (like Spring framework) as soon as it gets available, or would it be better to update all Spring dependencies at once as soon as a new Spring Boot version is available?

Holger Hagen (Unlicensed)

Sascha Frevel will create a ticket.

https://taskana.atlassian.net/browse/TSK-1258

Empty AttachmentController

The class AttachmentController is currently empty and has a single usage.
Can we and should we remove it?

Christopher Heiting (Unlicensed)

Delete the class.

Maven Profiles

What are the profiles used for? Are they all necessary? Can we name them better?

Mustapha Zorgati & Tobias Baden

Profile Id: coverage (Active: false , Source: pom)
Profile Id: eclipse (Active: false , Source: pom)
Profile Id: history.plugin (Active: false , Source: pom)
Profile Id: postgres (Active: false , Source: pom)
Profile Id: release (Active: false , Source: pom)
Profile Id: snapshot (Active: false , Source: pom)

Mustapha Zorgati will delete eclipse and postgres is renamed to wildfly-example.postgres

Documentation update bot

The bot asks, if the documentation has been updated. The agreed on response is a , which does not feel like an sufficient answer, if the documentation did not need updating.

Perhaps we should distinguish between “updated” and “not needed“.

Tristan Eisermann

Removal of asSummary()

Due to the new structure of the RepresentationModelAssembler classes the asSummary() method is no longer in use nor needed.

Should we delete it or reimplement it in the new structure?

Christopher Heiting (Unlicensed)

asSummary() can not be deleted since we need this method for the equals check: equals is a symmetrical function and would return false in case there is no asSummary().

Working with Forks / SonarQube

What is your experience working with a SonarCloud instance for each fork?

Currently I feel that we have a lot of overhead and can’t really trust the quality gate from the fork instance since the settings are not applied to it.

Depending on the result of the previous question:

• Should we change our process and work on Taskana/taskana instead of our forks?

• How can we improve further regarding code quality?

Mustapha Zorgati

We decided against the change of the working process since we prefer working with forks.

As a reviewer, check the link to sonarcloud analysis and possibly do not approve.

In case it is merged, mark the new issue as resolved after merging.

New Select And Claim API

How would you prefer the new select and claim API to be implemented:

1. New method on the Taskservice-Interface selectAndClaim with a TaskQuery parameter

2. Directly inside the TaskQueryImpl single() method as a special case

Advantage case 1: All other write operations that modify a task are offered through the Taskservice. → We stay consistent with the current programming model

Disadvantage case 1:This scenario needs to be performed in two steps: creating a query (and not executing it) and call the selectAndClaim method. Might feel less comfortable as a developer.

Advantage case 2: This scenario can be performed in one single step: Create and execute the TaskQuery as usual.

Disadvantages: The TaskQuery is not meant to perform write operations, as the name already suggests

joerg.heffner

After voting, we decided for option 2.

Exceptions in JunitTests

currently we have two ways of dealing with Exceptions in Unit Tests:

• list all Exceptions individually

• make the test throw Exception.class

We should unify this and make the result a guideline.
In Order to get an Idea use the following regex Expressions to find examples:

• .* throws (\w+Exception,? *)+ \{, file mask: *Test.java

• .* throws Exception \{, file mask: *Test.java

Mustapha Zorgati

Instead of listing all exceptions individually, we will throw Exception.class.

https://taskana.atlassian.net/browse/TSK-1318

Dependency management for often used dependencies

Currently we are managing our dependency versions in properties of the root pom. What if we have dependencies used by every single module, like junit: Should we keep them in every module pom, or start creating a pom hierachy to manage those dependencies better.

Is this even something to discuss, or am I overthinking it?

Mustapha Zorgati

We won’t change the current dependency management.

Autowiring Constructors

Should we use the annotation @Autowired for our autowired constructors or not?

Mustapha Zorgati

We use @Autowired annotation.

String constants for debug messages

Sonarcloud reports a duplicate warning if we use the same message in our logger statements.

Do we really want to use constants?

Example: https://sonarcloud.io/project/issues?id=Taskana_taskana&issues=AW_IAT9syzMGlbS0lw4f&open=AW_IAT9syzMGlbS0lw4f

Mustapha Zorgati

We won’t use constants in logs.

However, Benjamin Eckstein (Unlicensed) pointed out that this is a hint on code duplication.

Personal feelings about code in PRs

In our Code Review guideline we said that personal feelings about clean code are not part of the review.

In practice I experienced that the clean code tips I got from the team are very valuable in order to grow.

What do you think? Should we handle this point differently during reviews?

Suggestion: Prefix personal feelings with PERSONAL: and make that change optional, not mandatory?

Mustapha Zorgati / joerg.heffner

We allow personal feelings about clean code in our reviews whereby we will consider these comments as suggestions. Submitter and reviewer should agree on a solution. If you consider the result valuable, share it with the colleagues in the community of practices. Furthermore, if submitter and reviewer can not agree on a solution, discuss about the issue in the CoP.

Types in Lambdas

Should we, in order to increase readability, enforce types in our lambda statements?

example Backend:

(String sortBy, SortDirection sortDirection) -> {
    // bla bla
} 

over

(sortBy, sortDirection) -> {
    // bla bla
}

example Frontend:

this.categoryService.getClassificationCategoriesByType().pipe(
  take(1),
  tap(classificationTypes: CategoriesResponse => {
    // bla bla
  }),
);

over

this.categoryService.getClassificationCategoriesByType().pipe(
  take(1),
  tap(classificationTypes => {
    // bla bla
  }),
);

Note Chi Nguyen (Unlicensed) : with the coming styling PR this wouldn’t be needed since the IDE will do it automatically

Mustapha Zorgati

We decided to not enforce types in our lambda statements.

Variable naming conventions

Do we need naming conventions for all kind of variables? (parameters, instance variables, class variables … )

Mustapha Zorgati

The variable names have to be readable and understandable. There are no further restrictions or conventions except that.

End-to-End testing

Our current tests do not test the endresult our product produces. This leads to interesting problems, as it is possible, that all tests are successful, but the frontend wont show you anything but errors. ( Wrongly named selectors lead to this, as they do not get tested )

In our PRs we do not require manual testing, which could lead to a situation, where our bluemix will be completly unavailable.

I do not suggest we implement thorough e2e testing, but crude and unrefined automated testing could be beneficial in avoiding this.

Tristan Eisermann

We decided for e2e testing. Tristan and Jörg will integrate Tobias' tests after the next release.

Preparation for more users

Currently we have three major problems while developing:

• Structure: TASKANA grew historically. We always added functionality without properly reviewing its structure and reworking on it. This results in classes (TaskServiceImpl I am looking at you) which are way too big and complex

• Tests: All our tests are integration tests. This is not what integration tests are used for. We once talked about our test structure. Currently it is extremly annoying to write tests since all our tests are dependent from each other.

• Documentation: Currently our documentation is poor (from the user and technical perspective). With increasing demand of TASKANA we should start worrying about our documentation Structure to improve usability and increase productivity. An idea would be arc42.

Is this the right time to take a step back and start preparing for new feature sets and users?

Mustapha Zorgati

The documentation has the highest priority. We want to talk about changes of the wiki and javadoc in the next backlog refinement (10.08.).

Concerning the structure, we want to rework the TaskServiceImpl class:

The tests have the lowest priority.

String.format() or concatinations?

Currently both are used in the backend. Should one be preferred or avoided over the other?

Aspects we may want to consider:

• Localisation

• Performance

• Readability

Tristan Eisermann

Since String.format() makes code more readable but decreases performance, we prefer String.format() if the code is not constantly executed. Additionally, we don’t want to use String.format() in logging statements.

Module taskana-simpehistory-rest-spring-example

Do we really need this module?

Mustapha Zorgati / joerg.heffner

Do we want to use the assertJ soft assertion feature?

AssertJ offers a way to softly assert everything and get a detailled error message for every error and not fail for every single assertion. https://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#soft-assertions

I see that this can be useful when working with complex datastructures (like reports).

Mustapha Zorgati

When we will write new tests or edit old ones, we want to use the assertJ soft assertion feature.

Separate logic from assertions?

We have a testcase where I did the following assert:

assertThat(taskService.getTask(taskId).getState()).isEqualTo(TaskState.CLAIMED);

Should we consider always separating the business logic calls, to keep the asserts as small as possible? Does this make it more readable? Example:

String eventType = historyService.getHistoryEvent(listEvents.get(0).getId()).getEventType();

assertThat(eventType).isEqualTo("TASK_CLAIMED");

Should we make a general decision or should we say e.g. if the assert with bussiness logic fits into 1 LOC it’s fine and readability should be implied?

joerg.heffner

We only want to introduce a new variable if the number of operations in an assertion / expectation (backend / frontend) exceeds 2 or 3. This is not a fixed rule yet - we want to evaluate this approach in a few weeks.

Additionally, Mustapha pointed out that assertJ offers a fluent API with the purpose to achieve better readability. Therefore, methods like the extracting-method should be preferred if it’s possible.

Upgrade to Java 11

While upgrading to Java 11 there were several issues, for example:

• Postgres doesn’t support microsecond precision for Instants, while Instant.now() will return microsecond precision from Java 9 and on.

• The Group Interface is deprecated

I would like to present some possible solutions and discuss other ideas

joerg.heffner

Since Postgres does not support microsecond precision for Instants, we will truncate the Instants in the setters of all entities.
GroupPrincipal will implement Principal instead of the Group Interface which is deprecated from Java 9.
Furthermore, we will use Travis to ensure that both Java 8 and 11 work.

Behaviour of the single method in queries

joerg.heffner discovered an issue in the query api: https://taskana.atlassian.net/browse/TSK-1355

I would like to discuss this briefly with you to make sure that we all have the same understanding.

Holger Hagen (Unlicensed)

We decided for the most common solution as Holger mentions here. The one who calls the query must be sure that only one object will be returned. Otherwise, an exception will be thrown. We have to make sure that our java doc is correct.

Module folder structure

Personally I think that the module folder structure is not very good.
Especially regarding expansion in TSK-1277 (taskana-test-common).

Does anyone have a different idea?

Mustapha Zorgati

We will add a ‘common’ folder on the lib / rest / history level. This new folder will contain taskana-common-test and taskana-data. Taskana-data will be renamed to taskana-common-data. With these changes we want to improve readability and prevent the growth of the lib folder. We will not move all examples to an ‘examples’ folder since the examples should be in the same folder as the corresponding code.

Engage as Maintainers in Hacktoberfest

Hacktoberfest is a month long celebration for open source software.
During October developers and companies are asked to contribute to open source projects. When someone reaches 4 PRs in a hacktoberfest project they’ll get a free t-shirt.

Should we engage with this event and open TASKANA for the community in hope that someone will contribute and expand our reach?

Mustapha Zorgati

Mustapha Zorgati will do the organization so Taskana can participate. Mustapha, Holger and Jörg will collect simple tickets which are made available to the hacktoberfest project.

Possible fix for our auto-comment bot

I have a solution for the auto-comment bot. But I want to talk about an alternative to a bot:

pull request templates. With this we might achieve the same thing as the bot, but don’t rely on a third party system anymore.
Further details: https://docs.github.com/en/free-pro-team@latest/github/building-a-strong-community/about-issue-and-pull-request-templates#pull-request-templates

Mustapha Zorgati

We decided to use the pull request template. Ticket incoming.

Truncating of all Instant types

THIS IS CURRENTLY BLOCKING OUR MIGRATION TO JDK11. Let’s please prioritize this

Recently we discussed that we want to truncate all our Instants to microseconds in order to keep database compatiblity. Our first attempt was to modify the behaviour of our Pojo’s. I have an alternative solution. Let us talk about it please

Mustapha Zorgati

We agreed on keeping the way we discussed earlier and modify the Instant in our POJOs.

Constants in our REST-Mappings

In our mapping for the REST endpoints we have several constants that get put together from other constants and additional individual content.

Example:

public static final String PRE = "/api/v1/";

public static final String URL_CLASSIFICATIONS = PRE + "classifications";

public static final String URL_CLASSIFICATIONS_ID = URL_CLASSIFICATIONS + "/{classificationId}";

Should we keep it like it is or change the constants to not contain other constants in favor of a better readability?

joerg.heffner

We agreed on expanding our Strings in order to increase readibility

https://taskana.atlassian.net/browse/TSK-1430

Rename DocumentationTests?

Currently all our test classes which generate the Documentation are called XXXDocumentation.

Should we rename them to XXXDocumentationTest?

Mustapha Zorgati

We will rename these test classes to XXXDocumentationTest.

https://taskana.atlassian.net/browse/TSK-1436

Rename master branch

There is a move in the internet that the name git branch master is bad (e.g master/slave) and thus should be renaimed. A lot of people startet naming their old master branch main .
I can’t find links which explain this movement a little bit.

Since this is no effort and github already changed the default branch name we can follow aswell. https://github.com/github/renaming

Mustapha Zorgati

We will not rename our master branch for now. We will talk about this again if the topic becomes more urgent.

Which artifacts to release

In the past we released a lot of artifacts by accident. This is probably caused by multiple reasons.. One reason might be where we declare the artifacts which should be released.

I want to show you where you can find the artifacts which are released and the strategy to define them. There are two options for this

• declare all artifacts which should be released

• declare all artifacts which should NOT be released

Currently we are using the second strategy. Personally I think that was a factor why we released.

Mustapha Zorgati

We will declare all artifacts which should be released.

https://taskana.atlassian.net/browse/TSK-1438

https://taskana.atlassian.net/browse/ADPT-112

Checked vs Runtime Exceptions

Modern best practices recommend avoiding checked exceptions to have cleaner code. Checked Exceptions are only recommended when recovery is possible. Also instead of dealing with different Exception classes add a type/code property to reduce needed catch blocks. This avoids using an all catcher (like Exception e).

Ergänzung - Meinung von Axel:

“ganz klare Tendenz weg von checked exceptions. Neuere Sprachen wie Kotlin implementieren das schon gar nicht mehr (mit guter Begründung: https://kotlinlang.org/docs/reference/exceptions.html#checked-exceptions).

Kurz zusammengefasst ist das Hauptproblem, dass die Verankerung der Exception in der Methodensignatur eine starke Kopplung innerhalb der Aufrufketten verursacht. Das ist besonders bei nicht mehr ganz kleinen Mengen an Code unangenehm und lässt auch gute Teams vor Refactoring zurückschrecken. In Java empfehle ich, eine checked exception sofort als RuntimeException zu wrappen.

Wenn ihr eine entwicklerfreundliche API entwerft, dann dokumentiert die Exceptions gut, ggf. mit minimaler Typhierarchie und einem Basistyp, und macht sie bitte alle unchecked.”

Benjamin Eckstein (Unlicensed)

We don’t want a general conversion to unchecked exceptions.

The decision between checked and unchecked exception should depend on the use case e.g. the ConcurrencyException and the AttachmentPersistenceException could be converted to unchecked exceptions. Additionally, we could summarize exceptions and add an identifier for the context e.g the NotFoundException could be extended with the field ‘component’ and then the specific expections like the TaskNotFoundException could be dropped.

Delete unused method?

AttachementHandler#augmentAttachmentsByClassification is not used anymore. Shall we delete it?

Method was introduced by Bernd Breier (Unlicensed) on 09.03.2020 in this commit during TSK-1143

Mustapha Zorgati

Mustapha and Jörg will look at this after the COP.

JDK11 - what does that mean for us?

We finally upgraded to jdk11 with backwards compatibility to jdk8. What does this exaclty mean for us?

Mustapha Zorgati

In the ‘getting started’ of our documentation should be a hint that we have to ensure downward compatibility to Java 8. So we have to pay attention that we don’t use functions which are only available in Java 11. https://taskana.atlassian.net/browse/TSK-1455

Update node version

Currently we are running on node v12. LTS is 14. Can we easily upgrade?

Mustapha Zorgati

Chi Nguyen (Unlicensed) will update the node version.
https://taskana.atlassian.net/browse/TSK-1453

Maven Wrapper

Should we get maven-wrapper for our repositories?
Is there something equivalent for node?

Mustapha Zorgati

We decided to introduce the maven-wrapper. This means we won’t have to install Maven on our local systems anymore and everyone uses the same Maven version. We have to document the wrapper in our ‘getting started’ documentation and Tim Gerversmann will link this wherever in the docu Maven is used. https://taskana.atlassian.net/browse/TSK-1456

JDK for demo environment

Currently our demo environment (cloudfoundry) is using jdk8 to run our application. Should we update this to jdk11 too?
Note: Our build pipeline is only testing wildfly with jdk8.

Mustapha Zorgati

Currently, we don’t update to jdk11 since there are no effects.

Rename CloudFoundry organization

Currently the organization is “NovaTec Consulting GmbH”. According to our company guidelines the t is not capital anymore. Can we easily change that?

Mustapha Zorgati

We don’t rename the organization.

Report Endpoints

The Naming of our reports is inconsistent. Can we tidy this up?

Mustapha Zorgati

There are inconsistencies in our URLs, Java entities and rest docu which should be removed.
https://taskana.atlassian.net/browse/TSK-1465

JavaDoc Guidelines

Everyone has a different style of writing javadoc comments. Should we start getting some guidelintes aswell?
Example for Exceptions: Some people start the sentence with an if , others with a when.

Mustapha Zorgati

If we see a language mistake or an inconsistency in the JavaDoc while working on a PR, we can change it in a separate commit. Additionally, we should be aware that JavaDoc is part of a review.
Mustapha collects good practices for JavaDoc and writes them in the guidelines. We also have to collect good practices for the frontend documentation.

Test structure consistency

+

Monolithic tests

Our test structure seems to be not quite consistent. For some tests we have a single Tests class for all kinds of use-cases and for others, there are numerous classes for different use-cases.

What do we prefer? Should we agree on a general rule?

One example where this might bloat up our tests is attached below:

+

We have some tests, that test a lot of different use cases in 1 single test. E.g. testing different IN or LIKE parameters for querys.

Is that okay for us? Or should we agree on putting each use case into its own test?

joerg.heffner

We want to discuss these two topics in a separate meeting (~ 60 min) to collect suggestions and find a solution.

The meeting will be on 26.11. and Jörg will prepare and moderate the meeting.

Add jest-when to web app

jest-when is a utility test library that helps writing tests for jest easier. It is classed as ADOPT according to the latest thoughtworks techradar. Instead of writing long mock functions to be called in tests, you can directly call

const fn = jest.fn()
when(fn).calledWith(1).mockReturnValue('yay!')
expect(fn(1)).toEqual('yay!')

when(fn)
.calledWith(1).mockReturnValue('yay!')
.calledWith(2).mockReturnValue('nay!')
expect(fn(1)).toEqual('yay!')
expect(fn(2)).toEqual('nay!')

https://www.npmjs.com/package/jest-when

Chi Nguyen (Unlicensed)

We decided to add jest-when since it will make the tests shorter. Adding the jest-when library does not mean that the current notation is not okay, so we don’t have to change all existing test files.