Reduction of linter issues

[KristjanESPERANTO]

I got rid of a bunch of linter issues. I don't have any problems with my mirror, but I also have a very simple configuration. Please test 🙂

[klaernie]

hm, something goes wrong on my end - there is only one singular event sent out by the node_helper, so my calendars on the mirror are empty.

I inserted a console.log(`sending ${events.length} events for calendarId ${calendarId}`); before sending the events in node_helper.js line 311, and this comes up in the log:

[28.02.2022 08:34.13.745] [LOG]   [CALEXT2] calendar:Anja >> Scanned: 77, Selected: 77
[28.02.2022 08:34.13.746] [LOG]   sending 1 events for calendarId 0
[28.02.2022 08:34.14.073] [LOG]   [CALEXT2] calendar:Anja Work >> Scanned: 548, Selected: 481
[28.02.2022 08:34.14.077] [LOG]   sending 1 events for calendarId 1
[28.02.2022 08:34.14.109] [LOG]   [CALEXT2] calendar:Andre >> Scanned: 31, Selected: 31
[28.02.2022 08:34.14.114] [LOG]   sending 1 events for calendarId 2
[28.02.2022 08:34.14.312] [LOG]   [CALEXT2] calendar:Andre Work >> Scanned: 446, Selected: 446
[28.02.2022 08:34.14.317] [LOG]   sending 1 events for calendarId 3
[28.02.2022 08:34.14.416] [LOG]   [CALEXT2] calendar:Tobi >> Scanned: 0, Selected: 0
[28.02.2022 08:34.14.423] [LOG]   sending 1 events for calendarId 4
[28.02.2022 08:34.14.426] [LOG]   [CALEXT2] calendar:Tobi Shared >> No data to fetch
[28.02.2022 08:34.14.606] [LOG]   [CALEXT2] calendar:Tobi Lernsax >> Scanned: 1, Selected: 1
[28.02.2022 08:34.14.615] [LOG]   sending 1 events for calendarId 6

To verify this problem is localized to this branch I checked out main again, and the problem is immediately gone. I'm gonna dig a bit deeper to see what I can find.

[klaernie]

okay, next step was a simple one: I forcefully disabled event deduplication in L248 ( inserting a false && ), which brought the events back. So the deduplication is accidentially reducing multiple hundreds of events into one event.

[klaernie]

found it: reverting 524a786 is the solution. The inside of that for loop is supposed to return to the caller of compareThem() if the events need to be reordered.

Replacing this loop with a closure means the closure will return instead of compareThem(), losing the comparison result in the process.

With this behaviour the return value of compareThem() is fixed to the value 0 (the signifier for the events are equal) by the fall-through return 0;

This then causes the final deduplication to consider all events as duplicates, reducing all calendars to the first event it finds.

[klaernie]

reverting 524a786 is the solution

Thanks for working it out!  I have undone that. But is there anything else we can do here besides deactivating the linter rule?

The docs don't particularly recommend one approach there. Array.prototype.some() looked promising, until I noticed that is only ever returns a boolean, which means one state too less for a tristate output needed in a comparison function. I think having that loop done "the old fashioned way" and disabling the linter rule for that line is the only way to go.

[KristjanESPERANTO]

disabling the linter rule for that line is the only way to go.

Okay, done 🙂

[klaernie]

I'm thoroughly impressed - you brought us down to only 4 distinct errors.

The dangerous evals are all required to interpret what the user configured, possibly as text - so another case to neuter the linter for a line ;)

The stacked ternary could be a

ev.isCancelled = item.hasOwnProperty("component") && item.component.getFirstPropertyValue("status") != null &&  item.component.getFirstPropertyValue("status").toUpperCase() ===  "CANCELLED";

or am I missing a finer point? (does js short-circuit that way?)

[KristjanESPERANTO]

The stacked ternary could be a ...

Looks good!

The dangerous evals are all required ...

I disabled the linter rule.

So, one last problem to take care of... 🚀

I don't understand the method completely yet, but I tried to solve it with this: 83c997e

[KristjanESPERANTO]

There are two messages in the linter logs that I can't explain. Do you have an idea?

Line 9 yq: error: argument files: can't open '': [Errno 2] No such file or directory: '

Line 3219: ERROR! Failed to call GitHub Status API!

[klaernie]

There are two messages in the linter logs that I can't explain. Do you have an idea?

Line 9 yq: error: argument files: can't open '': [Errno 2] No such file or directory: '

For that one I probably will have to take a look at the config file processing superlinter does for the .yamllint. looks to me like a bug in superlinter

Line 3219: ERROR! Failed to call GitHub Status API!

That one seems a bit strange, but this bug report points a bit towards our trigger configuration for superlinter.

I'm going to update my checkout of this branch and see if everything is peachy

[klaernie]

I don't understand the method completely yet, but I tried to solve it with this: 83c997e

Totally overlooked that comment. My understanding of that function is that it allows you to build custom notifications inside CX2 and call them from other modules. But that's is just a rough guess, and I cannot imagine a use case for it.

[klaernie]

I've run this branch in production for 5 days straight, and I haven't seen any issue.
I'm still trying to figure out what the two error message kinds in superlinter are, but I don't think they are relevant to merging this branch.

So if you, @KristjanESPERANTO, feel good about it I'd suggest to merge this!

[klaernie]

Looks like the last kind of error message only happens for the pull request event trigger. The Superlinter run against main was flawless and did not contain that error message about calling the GitHub API.

Thanks for merging at this huge amount of work! It's greatly appreciated!

[KristjanESPERANTO]

Thank you for your cooperation. I have learned a few things in the process 🙂

Then I suggest that we ignore the two messages for now and see if they occur again in the future.