Add "Action Items" as a construct to checkup

[checkupjs]

Add "Action Items" as a construct to checkup

[carakessler]

Since we are removing the concept of Priority from TaskClassification -> I would actually recommend nesting ActionItem under that.

So currently:

export interface TaskMetaData {
  taskName: TaskName;
  friendlyTaskName: TaskName;
  taskClassification: TaskClassification;
}

export type TaskClassification = {
  category: Category;
  priority: Priority;
};

export const enum Category {
  Insights = 'insights',
  Migrations = 'migrations',
  Recommendations = 'recommendations',
}

will become

export interface TaskMetaData {
  taskName: TaskName;
  friendlyTaskName: TaskName;
  taskClassification: TaskClassification;
}

export type TaskClassification = {
  category: string;
  type: TaskType;
  actionItemData?: ActionItemData;
};

export const enum TaskType {
  Insights = 'insights',
  Migrations = 'migrations',
  Recommendations = 'recommendations', // likely nuke this as action items are a better way of communicating this same thing
}

[scalvert]

This is a good start to this concept. I would, perhaps, take a different approach - one that is slightly more compositional based.

I'd opt to define an interface that would be complimentary to the Task interface (specific details TBD):

interface ActionableTask {
  readonly isActionable: boolean;
  readonly actions: [TaskName, string[]];
}

This would allow task authors to decorate their tasks with this interface. We could then introspect the task when running each task in the TaskList, and determine if it's actionable, and gather the actions there, similar to how we gather errors. This would allow us to build up the list of actions to be output after all tasks are run.

Of course, this proposal omits a number of details:

• What sort of data do we capture in actions?
• What does a single action's data structure look like?

But it does allow for a lot of flexibility in how one gathers and populates actions.

[carakessler]

We are aligned on extending the Task interface to create an ActionableTask. However, I think that actions will need more data than just an array of strings. I am proposing the following:

export interface Action {
  key: string;
  condition: Function;
  value?: number;
  message: Function;
}

export interface ActionableTask extends Task {
  readonly isActionable: boolean;
  readonly actions: [TaskName, Action[]];
}

example of an action would be:

let action = {
	key: 'numEslintDisables';
    condition: (numEslintDisables, value = 0) => numEslintDisables > value;
    value: 20; 
	message: (value) => `There should be no more than ${value} instances of 'eslint-disable', please work on cleaning up linting errors!`
}

If we wanted the Action interface to be a bit slimmer, and potentially just define the data it needs to render the Action, and not the data it needs to determine whether or not to show the action, we can just have Action be a key, value pair, with the key being the name of the action, and the value being the message rendered if the Action is added to the ActionCollection. I think its a bit cleaner (especially if there are going to be a bunch of Actions for a given task, for the Action itself to contain the data around when it would be shown, instead of just leaving that business logic to the Task, but that is just my 2cents :)

If we want to keep the data in the ActionCollection cleaner, we can also have 2 data structures here. A task would define a bunch of Actions in the manner I describe above (where Action contains a condition and a value), and there can be a function in the Result that evaluates the conditions, and then adds key value pairs to the ActionCollection.

[carakessler]

Another thought - evaluating the actions has much more to do with the result than the task itself - instead of defining the actions in the Task, and then having to pass them to the TaskResult to be evaluated, maybe this whole thing should take place in the result. So instead of ActionableTask extends Task, it would be ActionableTaskResult extends TaskResult with the same structures described above.

This mental model makes sense to me - the task itself isnt really what is actionable, rather the results are what we want to take action on

[scalvert]

So are you proposing having actions as part of the TaskResult itself? We could do this, but you'd have to extract the actions in order to output them. Maybe that would be fine though, as you may want to serialize them to JSON for the output?

[carakessler]

ya in that case, we would gather the actions in the getReporter function, as they would be a part of the taskResults