Minor code review suggestions #642
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
alexeagle
requested review from
brandjon,
lberki,
thundergolfer and
andyscott
as code owners
f0rmiga
approved these changes
Mar 8, 2022
|
Thanks! |
f0rmiga
added a commit
that referenced
this pull request
Mar 9, 2022
* feat: cpython toolchains for linux and macos Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: compile zstd if missing Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: buildifier Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: make python_repositories reproducible Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * rename: python_repositories -> python_repository Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: linter Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: make interpreter files publicly visible Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: add files to py_runtime Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Account for some platforms not having all versions * Added windows support to hermetic toolchains (#628) * Added windows support to hermetic toolchains * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Update python/repositories.bzl Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * refactor: simplify logic for release urls Also, added a helper target to print the release hashes. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: Provide a host platform alias (#635) * feat: Provide a host platform alias This lets users and repository rules access the interpreter for whatever host the repository is running on. * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: files excludes Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: macOS dislikes --recursive Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: buildifier issues Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Allow previous indygreg releases (#636) This gives us more python patch versions * fix: put back zstd support for older releases Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: hash calculator Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: use hermetic interpreter with pip_parse and pip_install Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: add missing attrs back for zstd Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: expose zstd attributes Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: normalize OS names Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: linting issues Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: support windows in the aliases Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: linting issues Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: windows python.exe instead of python3.exe Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: use consts for OS names Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: always use latest toolchain for test Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: expose versions.bzl Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * refactor: move toolchain tests out of private Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: acceptance tests for the toolchains Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: rewrite test in py to work on windows Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: README example Co-authored-by: UebelAndre <github@uebelandre.com> * fix: use toolchain to run acceptance tests Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: use matrix for acceptance tests Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: support acceptance_tests on windows Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: alias for pip Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix?: include call to windows cmd Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Fix windows acceptance tests (#641) * Fix windows acceptance tests * test * todo: remove Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * refactor: polishing Windows testing support Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: unset py2_runtime Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * rename: host -> resolved_interpreter Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: add reference to quirks in python-build-standalone Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * feat: allow a distutils.cfg to be passed Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: buildifier (again) Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * Minor code review suggestions (#642) * Minor code review suggestions * Apply suggestions from code review Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: depset concat Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> Co-authored-by: Alex Eagle <alex@aspect.dev> Co-authored-by: UebelAndre <github@uebelandre.com>
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.
No description provided.