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

Fix incorrect script for no-pylint-disable #178

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

ericvergnaud
Copy link
Contributor

Our current script returns false positives for stacked PRs.
Upgrading it to the latest one from ucx

Copy link

github-actions bot commented Dec 9, 2024

✅ 40/40 passed, 2 skipped, 1m47s total

Running from acceptance #244

Copy link
Collaborator

@gueniai gueniai left a comment

Choose a reason for hiding this comment

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

LGTM

@gueniai gueniai merged commit 95d006d into main Jan 13, 2025
8 of 10 checks passed
@gueniai gueniai deleted the fix-pylint-disable branch January 13, 2025 15:22
gueniai added a commit that referenced this pull request Jan 13, 2025
* Bump codecov/codecov-action from 4 to 5 ([#174](#174)). In this release, the Codecov GitHub Action has been updated from version 4 to 5, featuring several new arguments and improvements to the underlying Codecov Wrapper. The new arguments include `binary`, `gcov_args`, `gcov_executable`, `gcov_ignore`, `gcov_include`, `report_type`, `skip_validation`, and `swift_project`. Additionally, the `file` and `plugin` arguments have been deprecated and replaced with `files` and `plugins`, respectively. These changes will ensure that the Action receives updates more quickly and provide developers with more flexibility in configuring their coverage reports. Furthermore, several existing arguments have been updated and improved, enhancing the overall functionality and usability of the Codecov GitHub Action.
* Fixed incorrect script for no-pylint-disable ([#178](#178)). This release introduces an upgrade to the script used for checking the disabling of linters in new code, adopting the latest version from the ucx project. Previously, the script returned false positives for stacked pull requests, particularly with pylint checks. This release resolves the issue by employing a Python script, `no_cheat.py`, which takes a generated diff file as an argument to accurately detect instances of linter disablement. The updated script then outputs the instances to a file called `cheats.txt` and fails the commit if more than one line is present, ensuring that instances of linter disablement are properly identified. This enhancement significantly improves the precision of the linter disablement check for pull requests and ensures that pylint disables are accurately tracked and reported, regardless of stacked pull requests.
* Skip dataclassess fields only when `None` ([#180](#180)). In this release, the behavior of skipping dataclass fields has been modified to allow skipping a field only when its value is `None`. This enhancement enables the writing of empty lists, strings, or zeros, addressing issue [#179](#179). The `_marshal_dataclass` method, responsible for marshalling dataclasses, has been updated to replace the conditional statement `if not raw:` with `if raw is None:`. By checking for explicit `None` values, the new logic preserves empty lists, strings, or zeros and does not treat them as `None`, ensuring that these specific cases are not unnecessarily skipped. This change provides more control and precision when marshalling dataclasses in the open-source library.

Dependency updates:

 * Bump codecov/codecov-action from 4 to 5 ([#174](#174)).
@gueniai gueniai mentioned this pull request Jan 13, 2025
gueniai added a commit that referenced this pull request Jan 13, 2025
* Fixed incorrect script for no-pylint-disable ([#178](#178)). In this release, we have updated the script used in the `no-cheat` GitHub workflow to address false positives in stacked pull requests. The updated script fetches the base reference from the remote repository and generates a diff between the base reference and the current branch, saving it to a file. It then runs the "no_cheat.py" script against this diff file and saves the results to a separate file. If the count of cheats (instances where linting has been intentionally disabled) is greater than one, the script outputs the contents of the results file and exits with a non-zero status, indicating an error. This change enhances the accuracy of the script and ensures it functions correctly in a stacked pull request scenario. The `no_cheat` function, which checks for the presence of certain pylint disable tags in a given diff text, has been updated to the latest version from the ucx project to improve accuracy. The function identifies tags by looking for lines starting with `-` or "+" followed by the disable tag and a list of codes, and counts the number of times each code is added and removed, reporting any net additions.
* Skip dataclassess fields only when `None` ([#180](#180)). In this release, we have implemented a change that allows for the skipping of dataclass fields only when the value is `None`, enabling the inclusion of empty lists, strings, or zeros during marshalling. This modification is in response to issue [#179](#179) and involves adding a check for `None` before marshalling a dataclass field. Specifically, the previous condition `if not raw:` has been replaced with `if raw is None:`. This change ensures that empty values such as `[]`, `''`, or `0` are not skipped during the serialization process, unless they are explicitly set to `None`. This enhancement provides improved compatibility and flexibility for users working with dataclasses containing empty values, allowing for more fine-grained control during the serialization process.

Dependency updates:

 * Bump codecov/codecov-action from 4 to 5 ([#174](#174)).
@gueniai gueniai mentioned this pull request Jan 13, 2025
gueniai added a commit that referenced this pull request Jan 13, 2025
* Fixed incorrect script for no-pylint-disable
([#178](#178)). In
this release, we have updated the script used in the `no-cheat` GitHub
workflow to address false positives in stacked pull requests. The
updated script fetches the base reference from the remote repository and
generates a diff between the base reference and the current branch,
saving it to a file. It then runs the "no_cheat.py" script against this
diff file and saves the results to a separate file. If the count of
cheats (instances where linting has been intentionally disabled) is
greater than one, the script outputs the contents of the results file
and exits with a non-zero status, indicating an error. This change
enhances the accuracy of the script and ensures it functions correctly
in a stacked pull request scenario. The `no_cheat` function, which
checks for the presence of certain pylint disable tags in a given diff
text, has been updated to the latest version from the ucx project to
improve accuracy. The function identifies tags by looking for lines
starting with `-` or "+" followed by the disable tag and a list of
codes, and counts the number of times each code is added and removed,
reporting any net additions.
* Skip dataclassess fields only when `None`
([#180](#180)). In
this release, we have implemented a change that allows for the skipping
of dataclass fields only when the value is `None`, enabling the
inclusion of empty lists, strings, or zeros during marshalling. This
modification is in response to issue
[#179](#179) and
involves adding a check for `None` before marshalling a dataclass field.
Specifically, the previous condition `if not raw:` has been replaced
with `if raw is None:`. This change ensures that empty values such as
`[]`, `''`, or `0` are not skipped during the serialization process,
unless they are explicitly set to `None`. This enhancement provides
improved compatibility and flexibility for users working with
dataclasses containing empty values, allowing for more fine-grained
control during the serialization process.

Dependency updates:

* Bump codecov/codecov-action from 4 to 5
([#174](#174)).
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.

2 participants