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

Add more perl signatures to lib::OpenQA::Files and Git #4490

Merged
merged 2 commits into from Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 5 additions & 11 deletions lib/OpenQA/Files.pm
Expand Up @@ -2,7 +2,7 @@
# SPDX-License-Identifier: GPL-2.0-or-later

package OpenQA::Files;
use Mojo::Base 'OpenQA::Parser::Results';
use Mojo::Base 'OpenQA::Parser::Results', -signatures;

use Digest::SHA 'sha1_base64';
use Mojo::File 'path';
Expand Down Expand Up @@ -36,9 +36,7 @@ sub prepare {
return $self->each(sub { $_->prepare });
}

sub write_chunks {
my ($class, $chunk_path, $file) = @_;

sub write_chunks ($class, $chunk_path, $file) {
return Mojo::File->new($chunk_path)->list_tree()->sort->each(
sub {
my $chunk = OpenQA::File->deserialize($_->slurp);
Expand All @@ -47,24 +45,20 @@ sub write_chunks {
});
}

sub write_verify_chunks {
my ($class, $chunk_path, $file) = @_;
sub write_verify_chunks ($class, $chunk_path, $file) {
$class->write_chunks($chunk_path => $file);
return $class->verify_chunks($chunk_path => $file);
}

sub spurt {
my ($self, $dir) = @_;
sub spurt ($self, $dir) {
return $self->each(
sub {
$_->prepare; # Prepare your data first before serializing
Mojo::File->new($dir, $_->index)->spurt($_->serialize);
});
}

sub verify_chunks {
my ($class, $chunk_path, $verify_file) = @_;

sub verify_chunks ($class, $chunk_path, $verify_file) {
my $sum;
for (Mojo::File->new($chunk_path)->list_tree()->each) {
my $chunk = OpenQA::File->deserialize($_->slurp);
Expand Down
26 changes: 8 additions & 18 deletions lib/OpenQA/Git.pm
Expand Up @@ -3,29 +3,25 @@

package OpenQA::Git;

use Mojo::Base -base;
use Mojo::Base -base, -signatures;
use Cwd 'abs_path';
use OpenQA::Utils qw(run_cmd_with_log_return_error);

has 'app';
has 'dir';
has 'user';

sub enabled {
my ($self) = @_;
sub enabled ($self, $args = undef) {
die 'no app assigned' unless my $app = $self->app;
return ($app->config->{global}->{scm} || '') eq 'git';
}

sub config {
my ($self) = @_;
sub config ($self, $args = undef) {
die 'no app assigned' unless my $app = $self->app;
return $app->config->{'scm git'};
}

sub _prepare_git_command {
my ($self, $args) = @_;

sub _prepare_git_command ($self, $args = undef) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you added $args = undef here because the tests failed. However, it looks still wrong because if $args can be undef we should not unconditionally access a key of it like it is done in the next line. Since we're actually intentionally passing undef here in some cases you've found a bug. See my other inline comment for an easy fix.

my $dir = $args->{dir} // $self->dir;
Martchus marked this conversation as resolved.
Show resolved Hide resolved
if ($dir !~ /^\//) {
my $absolute_path = abs_path($dir);
Expand All @@ -34,25 +30,20 @@ sub _prepare_git_command {
return ('git', '-C', $dir);
}

sub _format_git_error {
my ($git_result, $error_message) = @_;

sub _format_git_error ($git_result, $error_message) {
if ($git_result->{stderr}) {
$error_message .= ': ' . $git_result->{stderr};
}
return $error_message;
}

sub _validate_attributes {
my ($self) = @_;

sub _validate_attributes ($self) {
for my $mandatory_property (qw(app dir user)) {
die "no $mandatory_property specified" unless $self->$mandatory_property();
}
}

sub set_to_latest_master {
my ($self, $args) = @_;
sub set_to_latest_master ($self, $args = undef) {
$self->_validate_attributes;

my @git = $self->_prepare_git_command($args);
Expand All @@ -70,8 +61,7 @@ sub set_to_latest_master {
return undef;
}

sub commit {
my ($self, $args) = @_;
sub commit ($self, $args = undef) {
$self->_validate_attributes;

my @git = $self->_prepare_git_command($args);
Expand Down