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

Set additional compatible vendors with the call SetAdditionalVendors #138

Merged
merged 15 commits into from
Oct 9, 2020

Conversation

schubi2
Copy link
Member

@schubi2 schubi2 commented Sep 16, 2020

No description provided.

@schubi2 schubi2 changed the title Master jira sle 15184 WIP: Master jira sle 15184 Sep 17, 2020
@schubi2 schubi2 changed the title WIP: Master jira sle 15184 Set additional compatible vendors with the call SetAdditionalVendors Oct 1, 2020
Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

Just some minor improvements...

src/Package.cc Show resolved Hide resolved
src/Package.cc Outdated
* @short set additional vendors which are compatible
* @description
* Select additional compatible vendors for installation.
* @param list<string> List of additional vondor strings
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param list<string> List of additional vondor strings
* @param list<string> List of additional vendor strings

src/Package.cc Outdated
*
* @short set additional vendors which are compatible
* @description
* Select additional compatible vendors for installation.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's mainly used during upgrade...

Suggested change
* Select additional compatible vendors for installation.
* Select additional compatible vendors for upgrade.

src/Package.cc Outdated

try
{
if ( zypp::getZYpp()->getTarget() ) {
Copy link
Member

Choose a reason for hiding this comment

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

Use the zypp_ptr() wrapper instead of calling zypp::getZYpp() directly.

Suggested change
if ( zypp::getZYpp()->getTarget() ) {
if ( zypp_ptr()->getTarget() ) {

src/Package.cc Outdated
if ( zypp::getZYpp()->getTarget() ) {
zypp::VendorAttr vendorAttr { zypp::getZYpp()->getTarget()->vendorAttr() };
vendorAttr.addVendorList(vendors);
zypp::getZYpp()->getTarget()->vendorAttr( std::move(vendorAttr) );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
zypp::getZYpp()->getTarget()->vendorAttr( std::move(vendorAttr) );
zypp_ptr()->getTarget()->vendorAttr( std::move(vendorAttr) );

@@ -686,6 +686,8 @@ class PkgFunctions
YCPValue GetSolverFlags();
/* TYPEINFO: boolean(map<string,any>)*/
YCPValue SetSolverFlags(const YCPMap& params);
/* TYPEINFO: void(list<string>) */
Copy link
Member

Choose a reason for hiding this comment

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

Then

Suggested change
/* TYPEINFO: void(list<string>) */
/* TYPEINFO: boolean(list<string>) */

* @usage Pkg::SetAdditionalLocales(["openSUSE","SUSE LLC"]);
*/
YCPValue
PkgFunctions::SetAdditionalVendors (const YCPList &vendorycplist)
Copy link
Member

Choose a reason for hiding this comment

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

Um, this is only "write" function. Could we also implement a read counterpart SetAdditionalVendors()? Just to make it symmetric. Sometimes it can be useful to know the current state.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm VendorAttr in libzypp has not such kind of call. Do you see it ?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

I do not want to block this PR, if adding a "read" function is easy just add it. Otherwise I'm fine with it, we can add it later when really needed.

* @usage Pkg::SetAdditionalLocales(["openSUSE","SUSE LLC"]);
*/
YCPValue
PkgFunctions::SetAdditionalVendors (const YCPList &vendorycplist)
Copy link
Member

Choose a reason for hiding this comment

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

@schubi2 schubi2 merged commit 7be3473 into master Oct 9, 2020
@schubi2 schubi2 deleted the master-Jira-SLE-15184 branch October 9, 2020 13:14
@yast-bot
Copy link
Contributor

yast-bot commented Oct 9, 2020

❌ Internal Jenkins job #17 failed

@yast-bot
Copy link
Contributor

yast-bot commented Oct 9, 2020

✔️ Public Jenkins job #34 successfully finished
✔️ Created OBS submit request #840430

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #18 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #19 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #20 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #21 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #22 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #23 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #24 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #25 failed

@yast-bot
Copy link
Contributor

❌ Internal Jenkins job #26 failed

@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #27 successfully finished
✔️ Created IBS submit request #228822

schubi2 pushed a commit that referenced this pull request Oct 23, 2020
* added SetAdditionalVendors
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.

3 participants