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

bs_publish: move out repository create stuff into separate function #7891

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhabacker
Copy link
Contributor

This refactoring provides the counterpart of deleterepo() to increase code readability.

see #7679

Please note that this is my first obs post in Perl and I am not 100% sure if I have always used the correct Perl syntax.

my $prp = "$projid/$repoid";
print " creating repository\n";
my ($extrep, $stageservers) = BSUrlmapper::get_extrep_stageservers($prp);
return unless $extrep;
Copy link
Member

Choose a reason for hiding this comment

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

you don't create repo metadata anymore, if no stage server is defined?
This is not wanted....

Copy link
Contributor Author

@rhabacker rhabacker Jul 12, 2019

Choose a reason for hiding this comment

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

Because $stageservers is unused here I assume that you are suggesting to use BSUrlmapper::get_extrep($prp) instead, as done in the updated patch ?

This refactoring provides the counterpart of deleterepo() to increase
code readability.

see openSUSE#7679
@mlschroe
Copy link
Member

Hey, where's the xrepoid handling? You can't simply delete such things...

@mlschroe
Copy link
Member

Oh, wait, it's there. The inline comments confused me.

@mlschroe
Copy link
Member

Please put $extrep in the data hash instead of calculating it again.

@mlschroe
Copy link
Member

And your createrepo is actually not the counterpart of deleterepo, the counterpart of deleterepo is in fact the publish function. Your createrepo is more like a createmetadata function.

@rhabacker
Copy link
Contributor Author

And your createrepo is actually not the counterpart of deleterepo, the counterpart of deleterepo is in fact the publish function. Your createrepo is more like a createmetadata function.

There is a set of createrepo_xxx() and deleterepo_xxx() function called by this new function. Does this mean that these functions are also only creating meta data and should be renamed to createmetadata_xxx and deletemetadata_xxx ?

@rhabacker
Copy link
Contributor Author

Furthermore there are

   createpatterns_ymp($extrep, $projid, $xrepoid, $data, $patterntype{'ymp'});
   deletepatterns_ymp($extrep, $projid, $xrepoid, $data);

inside these functions. Does your statement apply also to this ?

@hennevogel hennevogel added the Backend Things regarding the OBS backend label Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Things regarding the OBS backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants