coordination #96596
open
[qe-core][CI] CI/CD and Coding style improvements
72%
Description
Story¶
As a code reviewer I would like the CI to do menial checks so that I can focus on bigger details and be more effective with my work.
Background¶
Often when doing the reviews on GH, reviewers tend to mention when:
- File header not being updated
- File without summary and mantainer field
- Create a mantainer list and only allow a person to be mantainer in a module, if they're in such MANTAINERS
- Syntax checkers for coding style
- Things like if check_var('ARCH') instead of
is_aarch64being wildly used, When there isUtils::Architectures::is_aarch64. When finding the check_var syntax, propose the aforementioned function - Things like if check_var('BACKEND', 'qemu') instead of
is_qemubeing wildly used, When there areUtils::Backends::is_qemu. When finding the check_var syntax, propose the aforementioned function. - Mix between
my $self = shiftandmy ($self) = @_, the proposal would be to stick tomy ($self) = @_
- Things like if check_var('ARCH') instead of
- Failures in the tidy checks that are often hard to find (Meaning, can we get comments on the offending lines?), which can be maybe generic enough to help with other parts of automated review.
- Know which jobs would be possibly affected by certain code changes
- Adding openqa-module-mapper in the current state, display last 20 jobs that are related to the changes in the PR.
AC's for feach item¶
- AC1: Each subtask is proposed as an RFC in the os-autoinst-distri-opensuse repo.
- AC2: There's a corresponding entry in the contributing document.
For syntax checking (check_var, get_var, script.*zypper... etc)¶
- Create a hash table in yaml, json, xml, CORBA, toml, $MARKUPLANGUAGE to check with a regex for a syntax that when matching, would propose a suggestion.
- Use etherpad.nue.suse.com/qe-core-proposed-syntax-checks to add the proposals.
Updated by szarate about 2 years ago
- Description updated (diff)
Updated by szarate about 2 years ago
- Description updated (diff)
For getting modified files in the current changeset: git status --porcelain "${files[@]}" | awk '{ print $2 }'
Updated by szarate about 2 years ago
- Project changed from 175 to openQA Tests
- Category set to Infrastructure
- Target version set to QE-Core: Ready
Updated by
Updated by szarate over 1 year ago
- Related to action #51230: [qe-core][functional] Unify all usages of systemctl added
Updated by szarate about 1 year ago
- Related to action #115343: [qe-core] os-autoinst-distri-opensuse checks for record_soft_failure without valid reference is ineffective added
Updated by szarate about 1 year ago
- Related to action #116599: [qe-core] SCC regcodes leaking mitigation - wheels fail in CI Compile checks when enabled added
Updated by szarate about 1 year ago
- Tracker changed from coordination to action
Updated by szarate about 1 year ago
- Tracker changed from action to coordination
Updated by szarate about 1 year ago
- Subject changed from [qe-core][CI] Implement checks in the CI for common errors/coding style to [qe-core][CI] CI/CD and Coding style improvements
- Status changed from New to Blocked
- Assignee set to szarate
- Target version set to QE-Core: Ready
Updated by szarate about 1 year ago
- Parent task deleted (
#95161)
Removing parent for now, this is a separate epic on it's own
Updated by szarate about 1 year ago
This ticket should help with preparing of the automated deployments or rather the triggering of new Service Packs into Development or Maintenance phases, for this we need a new epic.
This epic should stay only for static checks and code improvements.
- automatic push of use strict; warnings and testapi goes in here too.
Updated by szarate 12 months ago
- Related to deleted (action #115343: [qe-core] os-autoinst-distri-opensuse checks for record_soft_failure without valid reference is ineffective)
Updated by szarate 9 months ago
I always ended up calling the stuff directly on my own and never bothered to go further, but having make targets that allow parameters makes so much more sense...
and having a target similar to make tidy, we can actually do the same with make tests so the behavior only affects modified files.
Updated by pvorel 9 months ago
From our discussion in RC linked in previous: It'd be great to have way to easily run all tests which are in CI (./tools/check_metadata, ./tools/tidy, ...) on single file or list of specified files. What I usually need is to quickly test 1-3 files. Make targets used for testing, which are specified in top level Makefile are run on all files, which does not scale.
There could be ./tools/test-everything, which would expect list of files and run all tests on them (or at least these which can be run easily locally). The problem is that some commands are defined in the top level Makefile (e.g. perlcritic), all of these would have to be also put as scripts in ./tools/. Having commands defined in Makefile can brings complications (at least you need to have make installed, I'd try to have minimum scripting in Makefile, just call scripts in ./tools/).
Not only I want fast way to test everything I modified locally. Other problem, which bothers me is the long output, when everything is tested with pipes. Pipes does not print affected file, thus I need to scroll to find which tool reported particular error and run it locally on files I modified to see the problem.
Running locally I get files:
./tools/check_metadata $(git ls-files)
Missing Copyright <year> SUSE LLC in data/ai_ml/tvm/post_processing.py
Missing '# Summary: <multi line summary of test>' in data/apparmor/adminer.php5
Missing '# Maintainer: <email address>' in data/apparmor/adminer.php5
But in CI I see many useless output, which makes things hard to find:
tools/check_metadata $(git ls-files "tests/**.pm")
# Maintainer: Guillaume GARDET <guillaume@opensuse.org>
# Maintainer: Guillaume GARDET <guillaume@opensuse.org>
# Maintainer: QE Security <none@suse.de>`
The same problem is with other tests perlcritic.