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

feat(query): add query verb with initial preset features #57

Merged
merged 16 commits into from
Jan 14, 2022

Conversation

mattem
Copy link
Member

@mattem mattem commented Oct 23, 2021

Add query functionality. Users can provide regular bazel queries that will be passed directly to bazel. Users can also take advantage of predefined queries that will simply prompt the user for bazel label(s) as needed

@JesseTatasciore JesseTatasciore force-pushed the feat/query_verb branch 2 times, most recently from 1dabb3e to 140ec6b Compare November 17, 2021 21:11
@JesseTatasciore JesseTatasciore force-pushed the feat/query_verb branch 2 times, most recently from 5642ce7 to 18fedf6 Compare January 4, 2022 18:18
@JesseTatasciore
Copy link
Member

Can pass through directly to bazel query:
passthrough-query

Can prompt the user to use a preset query when no arguments are given to query:
promt-for-preset

Can specify which preset query to use when calling query:
give-preset-name

Can specify a preset query and give its argument all in the same command:
give-full-preset

@JesseTatasciore JesseTatasciore marked this pull request as ready for review January 5, 2022 16:11
@alexeagle
Copy link
Member

"determine why a target depends on another" the prompts are "target" and "dependency" and at least to me, it's not clear which direction the arrow points.
Maybe the prompt could include placeholders, like "determine why target A depends on target B" and then "target A?" "target B?"

But, then in your next demo that won't work since aspect query why you wouldn't know what "A" and "B" refer to 🤔

Copy link
Member Author

@mattem mattem left a comment

Choose a reason for hiding this comment

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

Some notes off the top of my head for this:

  • Have we agreed on the preset format? Where does this live? I see we have both an .aspect and a plugins file. I generally dislike so many files in the repo root, and adding another one for presets just adds to that. Can we merge these?

  • There seems to be quite a large amount of boiler plate to add a verb, and we want presets for query, cquery and aquery. As an extension to this, it seems tricky to get the flags to pass though?

cmd/aspect/root/BUILD.bazel Outdated Show resolved Hide resolved
docs/help/topics/BUILD.bazel Outdated Show resolved Hide resolved
@JesseTatasciore
Copy link
Member

@alexeagle does something like this work better?
promt-for-preset-2

give-preset-name-2

@JesseTatasciore JesseTatasciore linked an issue Jan 5, 2022 that may be closed by this pull request
cmd/aspect/query/query.go Outdated Show resolved Hide resolved
cmd/aspect/query/query.go Outdated Show resolved Hide resolved
cmd/aspect/root/root.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query_test.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
@f0rmiga
Copy link
Contributor

f0rmiga commented Jan 14, 2022

@JesseTatasciore We have to keep our errors compliant with https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md.

Kinda hard to enforce but we should strive for it.

@f0rmiga
Copy link
Contributor

f0rmiga commented Jan 14, 2022

I think you have to rebase before merging because of my changes to docs generation.

pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
pkg/aspect/query/query.go Outdated Show resolved Hide resolved
@JesseTatasciore JesseTatasciore merged commit 55a9c57 into main Jan 14, 2022
@JesseTatasciore JesseTatasciore deleted the feat/query_verb branch January 14, 2022 18:50
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.

canned queries
4 participants