WIP | Refactor existing parser using typescript #164

Efode-r2d2 · 2024-06-06T12:38:33Z

No description provided.

Efode-r2d2 · 2024-06-06T12:40:32Z

@Midnighter this is work in progress(needs a lot of work and I am working on it).

Midnighter

The work seems in progress indeed and only small changes to the original so far. You have chosen to start with the perhaps the most complicated part of the application. Maybe we should pair up on changing it?

Midnighter · 2024-06-07T22:37:18Z

compose.override.yaml

@@ -8,6 +8,7 @@ services:
      - "127.0.0.1:${UI_PORT:-8000}:8000"
    volumes:
      - "./:/app"
+      - "/app/node_modules"


I have my doubts about this change. Could it be that you didn't rebuild the image after changing dependencies?

Midnighter · 2024-06-07T22:41:07Z

src/__tests__/constants.ts

Instead of using a long array and define everything in script, could we create pairs of files (markdown, JSON) perhaps, that we load in the tests? Seems easier to read and maintain to me.

Midnighter · 2024-06-07T22:43:25Z

package.json

@@ -16,6 +16,7 @@
    "format": "prettier --write src/"
  },
  "dependencies": {
+    "antlr4": "^4.13.1-patch-1",


Did you have a look at the TypeScript enabled antlr4 packages, are they any good?

Midnighter · 2024-06-07T22:44:09Z

src/__tests__/parser.spec.ts

Would change this then depending on what we do with the constants.ts.

Midnighter · 2024-06-07T22:44:50Z

src/infrastructure/service/parser.ts

@@ -0,0 +1,347 @@
+import { cloneDeep } from "lodash";


Typically, there is no more need for lodash when targetting ES6. We should try to get rid of this dependency.

Midnighter · 2024-06-07T22:50:42Z

src/infrastructure/service/parser.ts

+  return printer.adr;
+}
+
+export function adr2md(adrToParse) {


I'm not a huge fan of constructing the strings this way. Perhaps we can use template strings and some kind of fluid interface to construct markdown.

Or a markdown builder service where we add sections...

Midnighter · 2024-06-07T22:57:35Z

src/infrastructure/service/parser/classes.ts

I would consider ADR a domain model, but perhaps the parser rather works with a data transfer object, not sure yet.

Midnighter · 2024-06-07T22:59:04Z

src/infrastructure/service/parser/classes.ts

+        this.title = title || "";
+        this.status = status || "";
+        this.deciders = deciders || "";
+        this.date = date || "";
+        this.technicalStory = technicalStory || "";
+        this.contextAndProblemStatement = contextAndProblemStatement || "";
+        this.decisionDrivers = decisionDrivers || [];
+        this.highestOptionId = 0;
+        this.consideredOptions = [];
+        if (consideredOptions && consideredOptions.length > 0) {
+            for (let i = 0; i < consideredOptions.length; i++) {
+                this.addOption(consideredOptions[i]);
+            }
+        }
+        this.decisionOutcome = decisionOutcome || {
+            chosenOption: "",
+            explanation: "",
+            positiveConsequences: [],
+            negativeConsequences: []
+        };
+        this.links = links || [];
+
+        // Assure invariants for decisionOutcome attribute
+        if (!Object.prototype.hasOwnProperty.call(this.decisionOutcome, "chosenOption")) {
+            this.decisionOutcome.decisionOutcome = "";
+        }
+        if (!Object.prototype.hasOwnProperty.call(this.decisionOutcome, "explanation")) {
+            this.decisionOutcome.explanation = "";
+        }
+        if (!Object.prototype.hasOwnProperty.call(this.decisionOutcome, "positiveConsequences")) {
+            this.decisionOutcome.positiveConsequences = [];
+        }
+        if (!Object.prototype.hasOwnProperty.call(this.decisionOutcome, "negativeConsequences")) {
+            this.decisionOutcome.negativeConsequences = [];
+        }
+
+        this.cleanUp();


Constructors should be extremely simple. Rather have a separate factory method with more logic.

Efode-r2d2 · 2024-06-11T13:33:15Z

The work seems in progress indeed and only small changes to the original so far. You have chosen to start with the perhaps the most complicated part of the application. Maybe we should pair up on changing it?

Yeah, would love to pair up.

koppor · 2024-09-06T07:32:33Z

We finally managed to get MADR 4.0.0-beta released: https://adr.github.io/madr/ - in case there are no show stoppers, we will release that as 4.0.0.

Efode-r2d2 added 2 commits June 5, 2024 21:20

Minor fix to docker-compose file

70b5e79

Migrate exiting parser related files to typescript files

036a31f

Efode-r2d2 requested a review from Midnighter June 6, 2024 12:38

Define a new model for ArchitecturalDescisionRecord

15289dc

Midnighter reviewed Jun 7, 2024

View reviewed changes

Efode-r2d2 added 3 commits June 11, 2024 15:16

Define ADR and DecisionOutcome models

1f84a84

Configure eslint

bc26e22

CLEANUP MADRLexer

0187367

Define ConsideredOption class

10601e4

koppor mentioned this pull request Aug 26, 2024

Discuss "Good, because" vs. "Pros:" adr/madr#109

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP | Refactor existing parser using typescript #164

WIP | Refactor existing parser using typescript #164

WIP | Refactor existing parser using typescript #164

Are you sure you want to change the base?

WIP | Refactor existing parser using typescript #164

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment