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

Adjust the code in rel-10_0 branch to work with CodePolicy. #760

Closed
svenoe opened this issue Jan 28, 2021 · 8 comments
Closed

Adjust the code in rel-10_0 branch to work with CodePolicy. #760

svenoe opened this issue Jan 28, 2021 · 8 comments
Labels
tidying Tidying of the code
Milestone

Comments

@svenoe
Copy link
Contributor

svenoe commented Jan 28, 2021

While introducing and adapting CodePolicy to our likings there will be code changes on OTOBO at the same time. See also https://github.com/RotherOSS/CodePolicy/issues/2

@svenoe svenoe added the tidying Tidying of the code label Jan 28, 2021
@svenoe svenoe added this to the OTOBO 10.0 milestone Jan 28, 2021
@svenoe svenoe changed the title Adjust code to work with CodePolicy. Adjust the code to work with CodePolicy. Jan 28, 2021
svenoe pushed a commit that referenced this issue Jan 28, 2021
svenoe pushed a commit that referenced this issue Jan 28, 2021
svenoe pushed a commit that referenced this issue Jan 28, 2021
svenoe pushed a commit that referenced this issue Jan 28, 2021
bschmalhofer added a commit that referenced this issue Feb 5, 2021
so that the final 1; is not part of the POD
bschmalhofer added a commit that referenced this issue Feb 5, 2021
bschmalhofer added a commit that referenced this issue Feb 6, 2021
';' used in the element list of a join
bschmalhofer added a commit that referenced this issue Feb 6, 2021
Add some TODO comments for open questions.
@bschmalhofer
Copy link
Contributor

bschmalhofer commented Feb 6, 2021

The Perl::Critic failures are now down to eight files.
Determined with ../CodePolicy/bin/otobo.CodePolicy.pl --plugins +TidyAll::Plugin::OTOBO::Perl::PerlCritic -a 1>critic.out
There are some case I don't understand. The relevant lines have been marked with TODO-comments:

Kernel/System/MigrateFromOTRS/CloneDB/Driver/Base.pm
Kernel/System/MigrateFromOTRS/OTOBOCopyFilesFromOTRS.pm
Kernel/System/MigrateFromOTRS/OTOBOOTRSConnectionCheck.pm
Kernel/System/UnitTest/RegisterDriver.pm
Kernel/System/UnitTest/Selenium.pm
Kernel/System/UnitTest/Selenium.pm
Kernel/System/Web/Exception.pm
scripts/backup.pl

These cases have been fixed by changes in the CodePolicy. The TODO-comments have been removed.

bschmalhofer added a commit that referenced this issue Feb 6, 2021
@bschmalhofer
Copy link
Contributor

The policy https://metacpan.org/pod/Perl::Critic::Policy::ValuesAndExpressions::ProhibitCommaSeparatedStatements might hint at real errors. Make sure that there are no violations of that policy.

@bschmalhofer
Copy link
Contributor

Only a single Perl::Critic violation is left now. This violation is covered by https://github.com/RotherOSS/CodePolicy/issues/30.

@bschmalhofer
Copy link
Contributor

There are still some violations from OTOBO::Perl::ObjectDependencies;


[checked] Kernel/System/MigrateFromOTRS/CloneDB/Backend.pm
�[33mTidyAll::Plugin::OTOBO::Perl::ObjectDependencies�[0m
�[31mThe following objects are used in the code, but not declared as dependencies:
    'Kernel::Language',
�[0m

bschmalhofer added a commit that referenced this issue May 8, 2021
bschmalhofer added a commit that referenced this issue May 8, 2021
bschmalhofer added a commit that referenced this issue May 17, 2021
Issue #760: ignore Perl::Critic warnings fo InputOutput::RequireBriefOpen
bschmalhofer added a commit that referenced this issue May 23, 2021
bschmalhofer added a commit that referenced this issue May 23, 2021
Mostly where Perl::Tidy did not commpute length of UTF-8 chars correctly.
But accept them anyways, as the alignment is not that important.
bschmalhofer added a commit that referenced this issue May 23, 2021
It's a bit strange, but it does not matter for commented out code.
bschmalhofer added a commit that referenced this issue May 23, 2021
@bschmalhofer
Copy link
Contributor

Down to 3 failures:

bes:~/devel/OTOBO/otobo (issue-#760-satisfy_more_policies)$ ../CodePolicy/bin/otobo.CodePolicy.pl -a > policy_14.out

Error: 3 file(s) did not pass validation.
 - Kernel/System/Ticket/Event/NotificationEvent/Transport/Email.pm
 - Kernel/System/Calendar/Event/Transport/Email.pm
 - Kernel/System/MigrateFromOTRS/CloneDB/Backend.pm

The failures for the two Event modules are covered by https://github.com/RotherOSS/CodePolicy/issues/44. One failure still open.

bschmalhofer added a commit that referenced this issue May 25, 2021
for RotherOSS/CodePolicy:#44
bschmalhofer added a commit that referenced this issue May 25, 2021
bschmalhofer added a commit that referenced this issue May 25, 2021
bschmalhofer added a commit that referenced this issue May 25, 2021
bschmalhofer added a commit that referenced this issue May 25, 2021
The localised $Kernel::OM is used to generate a deviant database object.
This is not intuitive, but is a known workaround.
Also accept some suggestions from Perl::Critic.
bschmalhofer added a commit that referenced this issue May 25, 2021
Issue #760: accept violation of ObjectManagerCreation
@bschmalhofer
Copy link
Contributor

Accepted the violation of TidyAll::Plugin::OTOBO::Perl::ObjectManagerCreation in Kernel/System/MigrateFromOTRS/CloneDB/Backend.pm. This leaves two violations that are covered in an existion issue. Closing this issue when the PR for #1000 is merged and when there are no new violations.

@bschmalhofer bschmalhofer changed the title Adjust the code to work with CodePolicy. Adjust the code in rel-10_0 branch to work with CodePolicy. May 27, 2021
@bschmalhofer
Copy link
Contributor

Checked the Codepolicy for the rel-10_0 and the issue-#1000-zzz_reload branches. Looks good. Closing this issue and open one for the rel-10_1 branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tidying Tidying of the code
Projects
None yet
Development

No branches or pull requests

2 participants