Skip to content

refactor(status bar): extract status_bar module into a class

Tomas Vik requested to merge 228-prepare-for-detached-pipeline into main

We'll have to work on the status_bar.js logic when implementing #228 (closed).

You can have a quick glance at the current status_bar.js. This code has a few severe issues:

  • You'll notice a block of let statements at the beginning. That's a global state (one per the whole extension) and all methods in this module are changing this state.
    • This makes it extra hard to test the logic.
  • There is a large amount (22) of if statements for this ~220 LOC module and only one integration test (which is flakey #299 (closed) )
  • The methods communicate with each other by using the module variables (making the side effects hard to reason about).

What does this MR do

This extracts the module code into a StatusBar. The promise of this change is easier testing, fewer side effects, cleaning up before implementing #228 (closed) and maybe improving the flakiness of #299 (closed).

I didn't cover all 22+ branches of logic with tests, but I introduced enough unit tests to give me reasonable confidence that the refactoring didn't introduce regressions.

How to review the MR

This MR must be reviewed commit-by-commit. Otherwise the change would be too large to be understood.

After one commit introduces the StatusBar class, there is one commit for each method moved into the class. Those changes might look like the method got completely changed, but the only change was moving it into the class (indentation) and replacing the sb. reference with this.

Edited by Tomas Vik

Merge request reports