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

Fixes #13695,11612 - Shows ostree branches for a repo #5858

Merged
merged 1 commit into from
Mar 11, 2016

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Mar 8, 2016

This commit updates the base model for ostree and its repositories.
It also includes UI changes to display the branches available for an
ostree repo

<span translate>Manage Branches</span>
</button>


Copy link
Member

Choose a reason for hiding this comment

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

Extra empty line here

@parthaa parthaa force-pushed the branches-model branch 2 times, most recently from d465c12 to 5ff7d31 Compare March 9, 2016 20:56
@parthaa
Copy link
Contributor Author

parthaa commented Mar 9, 2016

@ehelms I have screenshots for the UI this PR adds https://partha.fedorapeople.org/ostree-prs/PR-5858/

@jlsherrill
Copy link
Member

To hide the '0 selected' just add:

<div data-block="selection-summary"></div>

@parthaa
Copy link
Contributor Author

parthaa commented Mar 9, 2016

To hide the '0 selected' just add:

Thanks!. added

@@ -0,0 +1,86 @@
describe('Controller: PackagesController', function() {
Copy link
Member

Choose a reason for hiding this comment

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

This is all about packages? I also don't see an OstreeController?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops !

@bbuckingham
Copy link
Member

@parthaa, in the screen shots, we have "OSTree Units". We discussed it in irc a little bit, but would we want to call it "OSTree Branch Versions", since that is what it equates to. (Note: it may be more verbose, but might resonate with users when correlating it to the 'Manage Branches'.

Thoughts or other opinions?

@bbuckingham
Copy link
Member

@parthaa, On the "Click on the Manage Branches" screenshot, should the user be able to select and remove branches? If not, should "Manage Branches" be changed to "View Branches" or something different. Or, is the intent to add the ability to manage them via a separate PR?

@@ -198,6 +198,8 @@ def resource_name(_i18n = true)
_("Docker Manifest")
when "Katello::DockerTag"
_("Docker Tag")
when "Katello::OstreeBranch"
_("Ostree Branch")
Copy link
Member

Choose a reason for hiding this comment

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

s/Ostree Branch/OSTree Branch/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeh its view only.

@parthaa
Copy link
Contributor Author

parthaa commented Mar 10, 2016

@parthaa, in the screen shots, we have "OSTree Units". We discussed it in irc a little bit, but would we want to call it "OSTree Branch Versions", since that is what it equates to. (Note: it may be more verbose, but might resonate with users when correlating it to the 'Manage Branches'.

Thoughts or other opinions?

That's a good question. A branch can have multiple versions. So the number before the units implies number of versions or branch versions as you pointed out.
@mccun934 @daviddavis @jlsherrill @ehelms - Which of the following do you vote for

  1. Keep em as Units repo list page and Manage Branches
  2. Keep em as Units repo list page but Manage Units and refer to them as units every where
  3. Branch Versions on repo list and Manage Branch Versions
  4. Just Versions everywhere
  5. Just Branches every where

They all sound acceptable . I prefer 1/2/5 ...

update_attributes(:name => json[:branch],
:version => json[:metadata][:version],
:commit => json[:commit],
:version_date => json["_created"].to_datetime
Copy link
Member

Choose a reason for hiding this comment

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

I assume uuid is not needed here, correct?

@ehelms
Copy link
Member

ehelms commented Mar 10, 2016

I guess i would equate this to how we speak of "packages" even though we deal with multiple versions of the same packages. We could assume the user will imply the same sort of relationship and settle on "Branches" everywhere would would via analogy equate to "Packages" and expect users to know Branches have versions (similar to other content).

@@ -0,0 +1,60 @@
<span page-title ng-model="repository">{{ 'Manage Ostree Branchesfor Repository:' | translate }} {{ repository.name }}</span>
Copy link
Member

Choose a reason for hiding this comment

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

s/Branchesfor/Branches for/

s/Ostree/OSTree/ ?

@bbuckingham
Copy link
Member

@parthaa, I agree with @ehelms and lean towards:

Just Branches every where

@jlsherrill
Copy link
Member

+1 to just 'branches' for me

@bbuckingham
Copy link
Member

@parthaa, I have reviewed and added some minor comments. ACK from me once updated and jenkins is happy.

This commit updates the base model for ostree and its repositories.
It also includes UI changes to display the branches available for an
ostree repo
@parthaa
Copy link
Contributor Author

parthaa commented Mar 11, 2016

@parthaa, On the "Click on the Manage Branches" screenshot, should the user be able to select and remove branches? If not, should "Manage Branches" be changed to "View Branches" or something different. Or, is the intent to add the ability to manage them via a separate PR?

@bbuckingham Renamed to View Branches

@bbuckingham
Copy link
Member

@parthaa, Thanks!

parthaa added a commit that referenced this pull request Mar 11, 2016
Fixes #13695,11612 - Shows ostree branches for a repo
@parthaa parthaa merged commit a0a214d into Katello:master Mar 11, 2016
@parthaa parthaa deleted the branches-model branch March 11, 2016 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants