Skip to content

Conversation

aahung
Copy link
Contributor

@aahung aahung commented Oct 28, 2020

Issue #, if available:

Description of changes:

Manage black in dev.txt, almost the same as aws/serverless-application-model#1748

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@aahung aahung force-pushed the better-black branch 4 times, most recently from 7eec9cb to 210b252 Compare October 28, 2020 21:39
@aahung aahung marked this pull request as ready for review October 28, 2020 21:39
@aahung aahung requested review from hoffa and hawflau October 28, 2020 21:40
@aahung aahung changed the title Better black Manage black version using requirement file Oct 28, 2020
hoffa
hoffa previously approved these changes Oct 28, 2020
.appveyor.yml Outdated
# Validate Code was formatted with Black
- "/tmp/black --check setup.py tests aws_lambda_builders"
# Validate Code was formatted with Black, black is only supported in Python 3
- if [[ $PYTHON_VERSION != "2"* ]]; then black --check setup.py tests aws_lambda_builders; fi
Copy link
Contributor

@hawflau hawflau Oct 28, 2020

Choose a reason for hiding this comment

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

is there any reason that you use $PYTHON_VERSION != "2"* but not $PYTHON_VERSION == "3"* or $PYTHON_VERSION -gt "2"*?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to if [[ $PYTHON_VERSION > "3" ]]; then black --check setup.py tests aws_lambda_builders; fi

Copy link

Choose a reason for hiding this comment

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

This might be pedantry, but the > operator compares lexicographically; is that the intention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

either lexicographically or numerically should be able to specify "python 3 or higher" right?

hawflau
hawflau previously approved these changes Oct 28, 2020
@aahung
Copy link
Contributor Author

aahung commented Oct 28, 2020

As discussed with @hoffa offline, with > "3" depends on the expected lexicographic order to be used, and there might be a chance this is used instead, which makes "2.7.14" > "3".

One variant applies to sequences of different lengths by comparing the lengths of the sequences before considering their elements. https://en.wikipedia.org/wiki/Lexicographic_order

@aahung aahung requested review from hoffa and hawflau October 28, 2020 23:10
@aahung aahung merged commit 842b95e into aws:develop Oct 29, 2020
mndeveci added a commit that referenced this pull request Jan 12, 2021
* Added Dynamic Encoding Selection Based on Windows System Default

* Fixed dotnet output system dependent encoding

* Updated test_custom_make urllib3 Version

* Reformatted with black

* Rollback Changes to popen for Python 2 Support

* Added Doc Page for DotNet Output Encoding

* fix(ruby): use stdout stream to raise exceptions from bundler

- relevant error information is in stdout instead of stderr for bundler.

* chore: bump version to 1.1.0

* chore: Upgrade pytest to 6.1.1

* chore: Upgrade mock to 4.0.2

* chore: Upgrade parameterize to 0.7.4

* chore: Upgrade coverage to 5.3

* chore: Upgrade flake8 to 3.8.4

* chore: Upgrade pylint to 2.6.0

* Consider using Python 3 style super() without arguments (super-with-arguments)

* Implicit string concatenation found in assignment (implicit-str-concat)

* Consider explicitly re-raising using the 'from' keyword (raise-missing-from)

* Only install backports.tempfile for 2.7 & 3.6

* python 2 fallbacks (pylint & mock)

* Revert "Consider using Python 3 style super() without arguments (super-with-arguments)"

This reverts commit 44284ea.

* Revert "Consider explicitly re-raising using the 'from' keyword (raise-missing-from)"

This reverts commit cd7c16e.

* Disable rules that affecting Python 2

* Manage black version using requirement file (#207)

* chore: Manage black version in dev.txt

* chore: Update appveyor, use black in pip

* chore: Add pre-commit-config

* chore: Format with black 20.8b1

* chore: Add make pr2.7 for Python 2 that does not run black

* chore: Remove python patch version in AppVeyor to be more robust (#213)

* chore: Remove biased language (#212)

* fix: run GlobalToolInstallAction in a lock block and only once (#214)

* fix: run GlobalToolInstallAction in a lock block and only once

* chore: change field name and update unit tests

* chore: black formatting

* fix: change tests to use threadpool for py2

* chore: update assert call and change usage of Executor

* chore: bump version to 1.2.0 (#218)

Co-authored-by: Wilton Wang <[email protected]>
Co-authored-by: Sriram Madapusi Vasudevan <[email protected]>
Co-authored-by: Sriram Madapusi Vasudevan <[email protected]>
Co-authored-by: Sam Liu <[email protected]>
Co-authored-by: _sam <[email protected]>
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