Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[puppy] Introduce a package for misc Dart CLI tools #335

Merged
merged 7 commits into from
Jan 10, 2025
Merged

[puppy] Introduce a package for misc Dart CLI tools #335

merged 7 commits into from
Jan 10, 2025

Conversation

kevmoo
Copy link
Member

@kevmoo kevmoo commented Jan 8, 2025

First tool: for_all_package_dirs to recursively run a command in every
directory that contains pubspec.yaml

@kevmoo kevmoo requested a review from devoncarew as a code owner January 8, 2025 23:18
@github-actions github-actions bot added package:corpus type-infra A repository infrastructure change or enhancement labels Jan 8, 2025
@kevmoo
Copy link
Member Author

kevmoo commented Jan 8, 2025

Will rebase once #334 lands!

@kevmoo kevmoo marked this pull request as draft January 8, 2025 23:18
Copy link

github-actions bot commented Jan 8, 2025

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ✔️
File Coverage

This check for test coverage is informational (issues shown here will not fail the PR).

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbols
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

@kevmoo kevmoo changed the title puppy [puppy] Introduce a package for misc Dart CLI tools Jan 8, 2025
First tool: for_all_package_dirs to recursively run a command in every
directory that contains pubspec.yaml
@kevmoo kevmoo marked this pull request as ready for review January 8, 2025 23:34
@kevmoo
Copy link
Member Author

kevmoo commented Jan 8, 2025

CC @jakemac53 RE convo in chat

Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

Some high level comments:

  • we may want a single entry-point here - to use puppy <command> and the package:args CommandRunner stuff
  • we may also want to leverage the pkgs convention - to not traverse into subdirectories arbitrarily
  • instead of an arbitrary 'run anything' command, should we instead have specific pub get / ... other commands?
  • if we do have a 'run anything' command, we'd probably want a shorter name than for_all_package_dirs; perhaps map (for map-reduce)?

@kevmoo
Copy link
Member Author

kevmoo commented Jan 9, 2025

Some high level comments:

  • we may want a single entry-point here - to use puppy <command> and the package:args CommandRunner stuff

Maybe? I lean towards specific top-level CLIs. They autocomplete where subcommands don't. Unless you have auto-complete magic installed.

  • we may also want to leverage the pkgs convention - to not traverse into subdirectories arbitrarily

I have cases where the package is NOT set up that way. I guess we could add a depth option or similar?

  • instead of an arbitrary 'run anything' command, should we instead have specific pub get / ... other commands?

Disagree. It's super nice to be able to run anything. Like dart run build_runner or similar

  • if we do have a 'run anything' command, we'd probably want a shorter name than for_all_package_dirs; perhaps map (for map-reduce)?

Agreed if we go w/ the one top-level thingy

@devoncarew
Copy link
Member

devoncarew commented Jan 9, 2025

  • we may want a single entry-point here - to use puppy <command> and the package:args CommandRunner stuff

Maybe? I lean towards specific top-level CLIs. They autocomplete where subcommands don't. Unless you have auto-complete magic installed.

I'd prefer a single entry-point - I think it scales better as we get more commands - but it's not a blocker for me.

  • we may also want to leverage the pkgs convention - to not traverse into subdirectories arbitrarily

I have cases where the package is NOT set up that way. I guess we could add a depth option or similar?

Can we just not support those repos? Or encourage the //pkgs/ convention?

I'm just hesitant of arbitrary recursion, and supporting nesting of nesting. Perhaps we stop looking after we've hit a pubspec dir? Or do that by default and have a flag to just keep going.

  • instead of an arbitrary 'run anything' command, should we instead have specific pub get / ... other commands?

Disagree. It's super nice to be able to run anything. Like dart run build_runner or similar

OK, makes sense.

  • if we do have a 'run anything' command, we'd probably want a shorter name than for_all_package_dirs; perhaps map (for map-reduce)?

Agreed if we go w/ the one top-level thingy

I like map :) But that's bikeshedding.

@jakemac53
Copy link
Contributor

I'm just hesitant of arbitrary recursion, and supporting nesting of nesting. Perhaps we stop looking after we've hit a pubspec dir? Or do that by default and have a flag to just keep going.

We should probably at least support example dirs, although if this is just for our own tooling then it might not really matter, because we don't really use that pattern outside of flutter.

@kevmoo
Copy link
Member Author

kevmoo commented Jan 10, 2025

How about default to no recursing deeper than the first discovered pubspec – unless you opt in

In my experience, it rarely causes issues...just (often) failures

@kevmoo kevmoo merged commit 25c57b8 into main Jan 10, 2025
14 checks passed
@kevmoo kevmoo deleted the puppy branch January 10, 2025 21:42
}
}

@CliOptions(createCommand: true)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to pipe through command aliases here? If we wanted to instead support puppy run?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh crap! I forgot! Just submit a PR to change that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-infra A repository infrastructure change or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants