-
Notifications
You must be signed in to change notification settings - Fork 21
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
process: sync commits from Aspect-internal silo #763
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
2 times, most recently
from
October 24, 2024 00:49
5ca530e
to
762f7e7
Compare
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
October 24, 2024 00:50
762f7e7
to
4f08e7a
Compare
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
October 24, 2024 00:55
4f08e7a
to
1b38b71
Compare
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
October 24, 2024 00:58
1b38b71
to
c2ceebe
Compare
…S the exception from Aspect CLI (standard) (#6930) Aspect CLI Pro is now just Aspect CLI and the open-source, open-core Aspect CLI is now Aspect CLI OSS. Also drop the ability to specify the version in `.aspect/cli/config.yaml` as this is not documented and not used anywhere currently. Aspect CLI versioning outside of `.bazeliskrc` needs more thought before we bring it back. --- - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Drop "Pro" from Aspect CLI name. The open-source Aspect CLI OSS now has the specifier. - Covered by existing test cases - Manual testing; please provide instructions so we can reproduce: ``` $ bazel-bin/cli/core/cmd/aspect/aspect_/aspect version Aspect CLI OSS version: unknown [not built with --stamp] $ bazel-bin/cli/core/cmd/aspect/aspect_/aspect --version aspect oss unknown [not built with --stamp] $ ./bazel-bin/cli/pro/pro_/pro version Aspect CLI version: unknown [not built with --stamp] $ ./bazel-bin/cli/pro/pro_/pro --version aspect unknown [not built with --stamp] ``` GitOrigin-RevId: eec53a03fdaa90a8e7401510e50059904de2b349
….<num_commits_in_week>-<commit_hash> (#6932) Aspect CLI will now be versioned independently from Workflows. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Aspect CLI will now be versioned independently from Workflows. Version stamp will follow monorepo versioning scheme `<year>.<week>.<num_commits_in_week>-<commit_hash>`. ### Test plan - Covered by existing test cases - Manual testing; please provide instructions so we can reproduce: ``` $ bazel-bin/cli/pro/pro_/pro version --aspect:lock_version Aspect CLI version: 2024.39.51-a6b3d0c7b $ bazel-bin/cli/pro/pro_/pro --version aspect 2024.39.51-a6b3d0c7b $ bazel-bin/cli/core/cmd/aspect/aspect_/aspect version --aspect:lock_version Aspect CLI OSS version: 2024.39.51-a6b3d0c7b $ bazel-bin/cli/core/cmd/aspect/aspect_/aspect --version aspect oss 2024.39.51-a6b3d0c7b ``` GitOrigin-RevId: cc67e6d0d579babe40b8eb43bad3f8d711556914
Minor enhancement to support the .gitignore trailing-/ so it only matches directories. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - New test cases added GitOrigin-RevId: 2711ceec5d8d5ffe9ec2f045b7c0994d4b0c5d6c
…js/.d.ts (#6967) The package.json `main`, `exports` and `types` normally reference the transpiled js or dts. Normally imports between .ts are all relative to the source ts so the `rootDir` and `outDir` aren't taken into account. When the package.json imports the transpioled .js/dts we must make the transpiled files available as well. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - New test cases added GitOrigin-RevId: 0613758a33636c5c8fb1f55425fa7f15ba1b5a04
The `js_library` and `npm_package` targets representing npm packages were sharing too much code and mixing `npm_package(srcs)` and `js_library(deps)`. --- - Covered by existing test cases - New test cases added GitOrigin-RevId: 9c03adbceff31bf1aecff18c9ca3373c71513262
Replacing the mix of constants with names and `is*` method names with only the methods. The one test change is essentially aligning with the default tsconfig value where tsx extensions are *not* preserved. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: 2fef14a6047d026f63abfa44c9612a1b040407ea
tsconfig `rootDir` filtering is now essentially opt-in via a `${rootDir}` template in the source globs, with the default globs opting in to keep the default behaviour. This means we no longer have this confusion about sources being filtered due to `rootDir` even though we might not be setting `ts_project(tsconfig, root_dir)` or might even be using `js_library` when there is nothing to transpile. This also avoids `filepath.Rel` for every single file, avoids looping over the files multiple times (collect + filter + group), fetching the tsconfig for each file etc. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - New test cases added GitOrigin-RevId: b8e64fc0b5ba7d71fa7cba83d3d4c97510a5fc7c
I have not been able to reproduce this in a standalone test but this fixes an issue in a client repo. I think the fix makes sense, the `npm_package(srcs)` attribute may be set at generation time for source files, but package targets also have dependencies that must be resolved+merged at resolution time. Essentially doing `SetAttr("srcs")` at generation and resolution time. Normally the `srcs` and `deps` are separate attributes, but for `npm_package` they are the same which is why this is a bit unique. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases - Manual testing; please provide instructions so we can reproduce: run on client repo GitOrigin-RevId: f316224b82d83d69f8858a74017f3e25f77f76bd
Things like package.json might import both the .d.ts and .js version of a file. Those can come from source .d.ts or .ts. --- ### Changes are visible to end-users: no ### Test plan - New test cases added GitOrigin-RevId: 500e11903f791b324a15c05ad2fca9a0a843a058
Feature request from Figma where they have a custom eslint.bzl and requesting the patches results in two actions on CI. Also fixes flag registration so they are "no-able" to match expected behaviour of other flags: `--nofixes`, `--noreport`, etc... --- - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes New `--fixes` flag added to lint command in the the Aspect CLI which defaults to true. Users can now set `--nofixes` to turn off requesting the lint patch output group (`rules_lint_patch`). Requesting the report outputs groups (`rules_lint_human` and `rules_lint_machine`) is still controlled by the `--report` and `--machine` flags. - Covered by existing test cases - New test cases added GitOrigin-RevId: 8351b619b16979d089dd600db9065d8cd48f699d
Close #530 ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: 91c1f483e9b74b668ac7690d52e543bb039f09ac
There are too many edge cases that are causing issues atm. This should have been optional or opt-in from the start. This makes these imports "optional", so the feature is still enabled by default and users upgrading the cli will get this new feature, but it should no longer ever cause errors. We can maybe change it to error when opted-in in the future and maybe change the default if we are more certain it handles most of the edge cases. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): yes - Suggested release notes appear below: yes Dependencies from package.json `main/types/exports` fields no longer cause errors when `aspect configure` can not resolve the referenced files. ### Test plan - New test cases added GitOrigin-RevId: 3199133ffd87902ab1906811d5383bb0de56b299
This no longer provides any useful information now that gazelle is patched to walk subdirectories instead of us doing it manually here. --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: 8cdd9e60d027f509099508b0e0b0a085afc0772d
Various test cases trying to reproduce a customer issue. --- ### Changes are visible to end-users: no ### Test plan - New test cases added GitOrigin-RevId: 0e04447116b665e83d89a0e91b9931631f5e44f4
Various test cases trying to reproduce a customer issue. --- ### Changes are visible to end-users: no ### Test plan - New test cases added GitOrigin-RevId: 382be45c9eaaac2578d8a85cc7df89a4f564a3f1
### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: 8f231d868477f4970ec9261edac355f3c157b0b5
…ct deps (#7060) An issue found at a client when upgrading from an ancient cli version to more recent. This changes the order of how dependencies can be resolved, reducing the precedence of tsconfig paths to be one of the lowest priorities instead of being on-par with relative file imports. The client use case and the test case added is when a tsconfig file has `paths` essentially just for the IDE dev experience or maybe for legacy non-bazel (or non-pnpm-workspace) reasons. Imports of local workspace projects should use `:node_modules/@wksp/project-a` via package.json instead of `//projects/a` vis tsconfig.json. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Pnpm workspace project dependencies resolved with `aspect configure` are now higher priority then resolving dependencies via tsconfig paths. ### Test plan - Covered by existing test cases - New test cases added GitOrigin-RevId: 7cf0f95489f5600001351d36a7f13500bc465b62
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins. Note that both of these are currently patched into gazelle while waiting for PRs which will allow the removal of all aspect code: bazel-contrib/bazel-gazelle#1921 bazel-contrib/bazel-gazelle#1908 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
…c (#7095) Package targets must be visible when linked by rules_js so public visibility is a good default. Close #758 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes `configure` generated package targets (`npm_package` or `js_library`) visibility is now set to public by default. ### Test plan - Covered by existing test cases GitOrigin-RevId: 0e6112b2af952a98de0dd6e80339e31bb98089e9
This is a note about the deprecated one (with `js_` prefix) referring to the new one (no `js_` prefix). ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases GitOrigin-RevId: b5a4c1fd7a51cd9b046b7b02978c2f2a709e132b
…--' (#7029) ### Changes are visible to end-users: no ### Test plan CI Co-authored-by: Greg Magolan <[email protected]> GitOrigin-RevId: 72ca500133f72b926bee8df1f476c928ea3ff0cd
Resolves high alerts: - https://github.com/aspect-build/silo/security/dependabot/99 - https://github.com/aspect-build/silo/security/dependabot/109 - https://github.com/aspect-build/silo/security/dependabot/113 --- ### Changes are visible to end-users: no ### Test plan - Covered by existing test cases CI GitOrigin-RevId: 42aea24fd80f10e671935af220d64e2246323788
See bmatcuk/doublestar#92 - each time a glob is executed it gets validated if it does not successfully match a path. This now validates it once up front and then avoids re-validating on every (non-matched) glob invocation. --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes `aspect configure` glob patterns are now validated once up front instead of per evaluation. ### Test plan - Covered by existing test cases GitOrigin-RevId: 82135c3f6342ea0893d5701b6bb4cfc451e6fe0b
jbedard
force-pushed
the
788EA150BB0FAAB4A3FF81F26DCBE189
branch
from
October 24, 2024 01:00
c2ceebe
to
8376606
Compare
gregmagolan
approved these changes
Oct 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit range https://github.com/aspect-build/silo/compare/cfa2a62823454674453f3d6458e52ed30fbe0581..eec53a03fdaa90a8e7401510e50059904de2b349