Skip to content

feat(analyze): add spinner for analysis phases#244

Open
dreyfus92 wants to merge 1 commit into
mainfrom
add/task-prompt
Open

feat(analyze): add spinner for analysis phases#244
dreyfus92 wants to merge 1 commit into
mainfrom
add/task-prompt

Conversation

@dreyfus92

@dreyfus92 dreyfus92 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

adds step by step progress during analyze so long runs don’t look frozen.

@github-actions

Copy link
Copy Markdown

⚠️ Duplicate Dependencies (found: 3, threshold: 1)

📦 Package 📋 Versions
eslint-visitor-keys
2 versions
  • @e18e/cli@0.0.1
    • eslint@10.5.0
      • @eslint-community/eslint-utils@4.9.1
        • eslint-visitor-keys@3.4.3

  • @e18e/cli@0.0.1
    • eslint@10.5.0
      • eslint-visitor-keys@5.0.1

@humanwhocodes/retry
2 versions
  • @e18e/cli@0.0.1
    • eslint@10.5.0
      • @humanfs/node@0.16.6
        • @humanwhocodes/retry@0.3.1

  • @e18e/cli@0.0.1
    • eslint@10.5.0
      • @humanwhocodes/retry@0.4.3

ignore
2 versions
  • @e18e/cli@0.0.1
    • eslint@10.5.0
      • ignore@5.3.2

  • @e18e/cli@0.0.1
    • typescript-eslint@8.61.1
      • @typescript-eslint/eslint-plugin@8.61.1
        • ignore@7.0.5

💡 To find out what depends on a specific package, run: npm ls example-package

@pkg-pr-new

pkg-pr-new Bot commented Jun 26, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@e18e/cli@244

commit: abd98b2

@dreyfus92 dreyfus92 requested a review from 43081j June 26, 2026 20:54
Comment thread src/analyze/report.ts
await runPlugins(context, plugins);
}

async function finalizeReport(context: AnalysisContext) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're not gaining much by extracting this. the function definition and call sites are about as many LoC as these two statements

Comment thread src/plugin-runner.ts
if (res.stats) {
updateStats(context.stats, res.stats, seenExtra);
}
await runPlugin(context, plugin, seenExtra);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing changed here right? other than it being extracted into a single-use function

probably unnecessary?

@dreyfus92 dreyfus92 Jun 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my initial thought was to encapsulate for readability purposes, but i think that you're right. it might be unnecessary. gonna fix this

Comment thread src/types.ts
manifest?: string[];
src?: string[];
categories?: ParsedCategories;
phased?: PhasedRunner;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is leaking internal implementation detail into the public options.

tbh im not sure we need a concept of a "runner" at all. that part may be overengineered. ill have a think and come back to it

Comment thread src/types.ts
run: () => Promise<void>;
}

export type PhasedRunner = (phases: ReportPhase[]) => Promise<void>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is whats not sitting right with me. it seems over-engineered that we need a concept of a "phased runner", phases, etc.

i would expect something much, much simpler:

options.interactive; // boolean, from Options

// elsewhere...

const trackProgress = (options, fn) => {
  if (!options.interactive) {
    return fn();
  }
  startSpinner();
  const result = await fn();
  stopSpinner();
  return result;
};

// elsewhere again...

await trackProgress(options, async () => {
  // do some stuff
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants