Skip to content

Conversation

isoos
Copy link
Collaborator

@isoos isoos commented Sep 10, 2025

  • Part of creating a shared/separate tool for creating screenshots in both pub-dev and dartdoc (Minimal screenshot tool. dartdoc#4074)
  • Dropped the PUB_ prefix from the environment variable, otherwise kept files unchanged.
  • Updated/expanded README.md.

@isoos isoos requested a review from sigurdm September 10, 2025 20:08
@isoos isoos force-pushed the screenshot-refactor branch from 983507e to dc12a49 Compare September 10, 2025 20:14
@@ -4,8 +4,8 @@
import 'dart:async';

import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should we change the name of the main library to align with the package name?

Suggested change
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:screenshot_tools/screenshot_tools.dart';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll do the library name changes in a follow-up, after more changes has been done, e.g. the dartdoc-related tooling is also implemented.

@@ -4,8 +4,8 @@
import 'dart:async';

import 'package:puppeteer/puppeteer.dart';
import 'package:screenshot_tools/screenshot_utils.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

And should we have the name of the new package indicate somehow what it is screenshotting?

Something like:

Suggested change
import 'package:screenshot_tools/screenshot_utils.dart';
import 'package:browser_screenshots/browser_screenshots.dart';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How about puppeteer_screenshots?

Copy link
Contributor

@szakarias szakarias left a comment

Choose a reason for hiding this comment

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

LGTM

@isoos isoos merged commit ae83d11 into dart-lang:master Sep 11, 2025
32 checks passed
@isoos isoos deleted the screenshot-refactor branch September 11, 2025 10:56
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.

3 participants