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

Fix registration #338

Merged
merged 5 commits into from Oct 11, 2017
Merged

Fix registration #338

merged 5 commits into from Oct 11, 2017

Conversation

jreidinger
Copy link
Member

No description provided.

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.

LGTM, just some wording suggestions

next_addon = to_process.find do |addon|
addon.depends_on.nil? || !to_process.include?(addon.depends_on)
end
raise "circular dependencies found in addons #{to_process.inspect}" unless next_addon
Copy link
Member

Choose a reason for hiding this comment

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

Report.Error would be probably more user friendly, but hopefully this never happens so let's make it simple...

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I hope we catch it quickly in openQA when this happen, so I hope not needed. Also Report is useless as result is that it won't work.

Copy link
Member

Choose a reason for hiding this comment

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

The problem is that it might happen after the release, few months later... But anyway as this is really a corner case let's use this simple solution...

@@ -78,6 +78,19 @@
end
end

describe ".registration_order" do
it "returns in addons in order that allow registration" do
Copy link
Member

Choose a reason for hiding this comment

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

returns addons sorted in the registration order

@@ -1,4 +1,11 @@
-------------------------------------------------------------------
Wed Oct 11 08:49:16 UTC 2017 - jreidinger@suse.com

- Fix registration when there is auto selected extensions
Copy link
Member

Choose a reason for hiding this comment

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

Fixed registration of autoselected extensions

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 69.34% when pulling fbfff2d on fix_registration into a146e54 on master.

@yast yast deleted a comment from coveralls Oct 11, 2017
@yast yast deleted a comment from coveralls Oct 11, 2017
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.

LGTM, thanks! 👍

next_addon = to_process.find do |addon|
addon.depends_on.nil? || !to_process.include?(addon.depends_on)
end
raise "circular dependencies found in addons #{to_process.inspect}" unless next_addon
Copy link
Member

Choose a reason for hiding this comment

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

The problem is that it might happen after the release, few months later... But anyway as this is really a corner case let's use this simple solution...

@jreidinger jreidinger merged commit a9c902c into master Oct 11, 2017
@jreidinger jreidinger deleted the fix_registration branch October 11, 2017 09:34
mvidner added a commit that referenced this pull request May 20, 2020
…c#1169577)

In commit 1c7d041,
PR #440 ,
the idea was that we can unselect an addon whose license was declined, and
proceed with installing the remaining addons. But that does not work out with
autoselected addons ( #335
#338 ) where we take into
account the product dependencies.

The new solution is to interpret a refusal of a license as the Abort button,
which ends installation of ALL selected addons.
mvidner added a commit that referenced this pull request May 20, 2020
…c#1169577)

Without this, an addon is still installed even if its license is declined.

In commit 1c7d041,
PR #440 ,
the idea was that we can unselect an addon whose license was declined, and
proceed with installing the remaining addons. But that does not work out with
autoselected addons ( #335
#338 ) where we take into
account the product dependencies.

The new solution is to interpret a refusal of a license as the Abort button,
which ends installation of ALL selected addons.
mvidner pushed a commit that referenced this pull request May 22, 2020
…c#1169577)

Without this, an addon is still installed even if its license is declined.

In commit 1c7d041,
PR #440 ,
the idea was that we can unselect an addon whose license was declined, and
proceed with installing the remaining addons. But that does not work out with
autoselected addons ( #335
#338 ) where we take into
account the product dependencies.

The new solution is to interpret a refusal of a license as the Back button,
which ends installation of ALL selected addons.
mvidner pushed a commit that referenced this pull request May 28, 2020
…c#1169577)

Without this, an addon is still installed even if its license is declined.

In commit 1c7d041,
PR #440 ,
the idea was that we can unselect an addon whose license was declined, and
proceed with installing the remaining addons. But that does not work out with
autoselected addons ( #335
#338 ) where we take into
account the product dependencies.

The new solution is to interpret a refusal of a license as the Back button,
which ends installation of ALL selected addons.
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.

None yet

3 participants