You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/05 10:00:38 UTC
[GitHub] [airflow] potiuk opened a new pull request, #24841: Remove "bad characters" from our codebase
potiuk opened a new pull request, #24841:
URL: https://github.com/apache/airflow/pull/24841
We had plenty of "bad characters" in our codebase that were not
invited and came here by accident. We want to get rid of those
"bad characters" once and for all.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a diff in pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #24841:
URL: https://github.com/apache/airflow/pull/24841#discussion_r913657077
##########
.gitmodules:
##########
@@ -1,12 +1,12 @@
[submodule ".github/actions/get-workflow-origin"]
- path = .github/actions/get-workflow-origin
- url = https://github.com/potiuk/get-workflow-origin
+ path = .github/actions/get-workflow-origin
Review Comment:
Or ignore the following files like we did before:
```
exclude: ^airflow/_vendor/|^clients/gen/go\.sh$|^\.gitmodules$
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24841:
URL: https://github.com/apache/airflow/pull/24841#discussion_r913683431
##########
.gitmodules:
##########
@@ -1,12 +1,12 @@
[submodule ".github/actions/get-workflow-origin"]
- path = .github/actions/get-workflow-origin
- url = https://github.com/potiuk/get-workflow-origin
+ path = .github/actions/get-workflow-origin
Review Comment:
we alredy had "forbid tabs" before - I just missed an exclusion (which I have to add anyway).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24841:
URL: https://github.com/apache/airflow/pull/24841#discussion_r913683784
##########
.gitmodules:
##########
@@ -1,12 +1,12 @@
[submodule ".github/actions/get-workflow-origin"]
- path = .github/actions/get-workflow-origin
- url = https://github.com/potiuk/get-workflow-origin
+ path = .github/actions/get-workflow-origin
Review Comment:
(this one will be better because it will fix them automatically).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174917190
What's wrong with using smart/curly quotes in docs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174970536
Updated much improved version:
1) brought back the same exclusions that "forbid tabs" used to have
2) No ruby/pre-commit dependency - all is pure python + rich (so there is really one virtualenv used for this and many other pre-commits - pre-commit nicely recognizes multiple pre-commits and will reuse the virtualenv that has the same set of requirements.
3) Nicer, colorful output informing what has been removed (I added "replacements" after the number comparing to the screenshot).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a diff in pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #24841:
URL: https://github.com/apache/airflow/pull/24841#discussion_r913656241
##########
.gitmodules:
##########
@@ -1,12 +1,12 @@
[submodule ".github/actions/get-workflow-origin"]
- path = .github/actions/get-workflow-origin
- url = https://github.com/potiuk/get-workflow-origin
+ path = .github/actions/get-workflow-origin
Review Comment:
Tabs are not bad characters -- flake8/black deals with this, so leave tabs alone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174954526
> What's wrong with using smart/curly quotes in docs?
For the context - this is the result of discussion in #24797 - where *nbsps* were shown in some IDES (only some IDES show it , some others will render it as space.
The main reason is consistency.
If we are using them inconsistently, then it's wrong. You either should use always curly quotes in your docs or always straight ones otherwise our docs will look strange if you look closely. Another problem it introduces it's "search & replace" functionality. Whenever you are using global search and replace and want to search for "quotes" you will often miss the curly quote cases - especially when it is "random" and not used consistently.
Most of the "curly ones" comes usually (similarly as *nbsp*) from copy-pasting from word-like sources, but some people will use the "straight" ones, some people will use the "curly" ones and it's difficult to keep consistency. And the bad thing with it is also that sometimes those curly quotes will sneak-in the code examples and snippets which are part of our documentation and we will not notice. This happened often to me where I wanted to copy some piece of code from the browser, It did not parse/compile or produce strange results.
Of course there are plenty exceptions where you realy can't use curly ones (code snippets etc.) and it's next to impossible to figure out automatically when yoiu `can` use them.. It's far easier to replace all curly ones with `straight` ones. The consistency of look&feel is kept and you avoid any of the problems above. And look & feel is marginally impacted IMHO.. And if we REALLY want to keep the curly quotes in some specific files, or folders we can always exclude them by regexp in pre-commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174871411
Turned out we had more "Bad characters" :P in our codebase than literally meets the eye :P
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a diff in pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #24841:
URL: https://github.com/apache/airflow/pull/24841#discussion_r913853042
##########
scripts/ci/pre_commit/pre_commit_replace_bad_characters.py:
##########
@@ -0,0 +1,73 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import re
+import sys
+from pathlib import Path
+from typing import List, NamedTuple
+
+from rich.console import Console
+
+if __name__ != "__main__":
+ raise Exception(
+ "This file is intended to be executed as an executable program. You cannot use it as a module."
+ "To run this script, run the ./build_docs.py command"
Review Comment:
nit: copy pasta error here.
```suggestion
f"To run this script, run the {__file__} command"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #24841:
URL: https://github.com/apache/airflow/pull/24841
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174913984
This is no go, I do not want to get ruby dependency for pre-commit. Time for 15-liner python script :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24841: Remove "bad characters" from our codebase
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24841:
URL: https://github.com/apache/airflow/pull/24841#issuecomment-1174912964
:( Executable `ruby` not found
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org