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

Jump to bottom

Remove pylint #16682

Merged
merged 1 commit into from Jun 28, 2021
Merged

Remove pylint #16682

merged 1 commit into from Jun 28, 2021

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 27, 2021 •
edited

We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

  • removes all # pylint comments
  • removes pylint pre-commits and related scripts/files
  • removes CI jobs running pylint checks
  • removes documentation about pylint
  • removes unnecessary #noga (adds pre-commit for that)
  • fixes some remaining pydocstyle errors after removing #noqa's

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:CLI area:dev-tools labels Jun 27, 2021
@potiuk
Copy link
Member Author

potiuk commented Jun 27, 2021

Anticipating the result of voting - I prepared this PR that removes pylint

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Jun 28, 2021
@uranusjr
Copy link
Member

We should add a .git-blame-ignore-revs file for this commit.

@uranusjr
Copy link
Member

(The changeset is so big GitHub is having trouble rendering, so I’m going to assume this is correct…)

@potiuk
Copy link
Member Author

potiuk commented Jun 28, 2021 •
edited

Indeed. Might be a good point also for few other changes like that. But this has to be a separate PR after we know the commit hash :)

@potiuk
Copy link
Member Author

potiuk commented Jun 28, 2021

I've added yesqa to pre-commits and removed all the unnecessary #noqa's. It removed a bit too many of those initially. It does not understand some pydocstyle problems but I figured it will be easier to remove a "public method no doc" error D103 | Missing docstring in public function (which I think is far too much to expect) and fixed remaining small docstyle errors which were earlier ignored by #noqa.

@potiuk potiuk mentioned this pull request Jun 28, 2021
Merged
@potiuk potiuk mentioned this pull request Jun 28, 2021
Closed
@potiuk
Removes pylint from our toolchain
1ba422a 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's
kaxil
kaxil approved these changes Jun 28, 2021
View reviewed changes
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

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

@potiuk potiuk merged commit 866a601 into apache:main Jun 28, 2021
@potiuk potiuk deleted the remove-pylint branch June 28, 2021 17:26
@potiuk
Copy link
Member Author

potiuk commented Jun 28, 2021

Booom

@potiuk potiuk mentioned this pull request Jun 28, 2021
Merged
@potiuk potiuk mentioned this pull request Jul 7, 2021
Closed
@uranusjr uranusjr mentioned this pull request Jul 9, 2021
Merged
jhtimmins pushed a commit to astronomer/airflow that referenced this pull request Jul 9, 2021
@potiuk @jhtimmins
Removes pylint from our toolchain (apache#16682)
064dd1c 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's

(cherry picked from commit 866a601)
potiuk added a commit that referenced this pull request Jul 9, 2021
@potiuk
Removes pylint from our toolchain (#16682)
c258646 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's

(cherry picked from commit 866a601)
jhtimmins pushed a commit to astronomer/airflow that referenced this pull request Jul 9, 2021
@potiuk @jhtimmins
Removes pylint from our toolchain (apache#16682)
fe060b5 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's

(cherry picked from commit 866a601)
jhtimmins pushed a commit that referenced this pull request Jul 9, 2021
@potiuk @jhtimmins
Removes pylint from our toolchain (#16682)
06df29e 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's

(cherry picked from commit 866a601)
cognifloyd pushed a commit to cognifloyd/stackstorm-k8s that referenced this pull request Feb 16, 2022
@potiuk @cognifloyd
Removes pylint from our toolchain (apache/airflow#16682)
ca283f7 
We've agreed during the voting process that Pylint support
should be disabled: https://lists.apache.org/thread.html/r9e2cc385db8737ec0874ad09872081bd083593ee29e8303e58d21efb%40%3Cdev.airflow.apache.org%3E

This PR:

* removes all # pylint comments
* removes pylint pre-commits and related scripts/files
* removes CI jobs running pylint checks
* removes documentation about pylint
* removes unnecessary #noga (adds pre-commit for that)
* fixes some remaining pydocstyle errors after removing #noqa's

Partial Commit Extracted From: https://github.com/apache/airflow
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mik-laj mik-laj mik-laj approved these changes

@ashb ashb ashb approved these changes

@kaxil kaxil kaxil approved these changes

@dimberman dimberman Awaiting requested review from dimberman

@ephraimbuddy ephraimbuddy Awaiting requested review from ephraimbuddy ephraimbuddy is a code owner

@jhtimmins jhtimmins Awaiting requested review from jhtimmins jhtimmins is a code owner

@ryanahamilton ryanahamilton Awaiting requested review from ryanahamilton ryanahamilton is a code owner

@turbaszek turbaszek Awaiting requested review from turbaszek

@vikramkoka vikramkoka Awaiting requested review from vikramkoka

@XD-DENG XD-DENG Awaiting requested review from XD-DENG XD-DENG is a code owner

Assignees
No one assigned
Labels
area:API Airflow's REST/HTTP API area:CLI area:dev-tools full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@potiuk @uranusjr @ashb @kaxil @mik-laj