Ostree remotes and rebasing #5599

petervo · 2016-12-15T20:27:55Z

No description provided.

petervo · 2016-12-15T20:37:58Z

Still a WIP because the design requires rebasing to be implemented in order for this to make sense to merge. Also needs tests.

@andreasn putting this up now so you can review while I'm working on rebasing. I had to change the add form a little bit. I added the name field and removed the gpg data field. There was a issue with the design if the add succeed but the import failed, so I figured it was ok to make that part of the edit step only. But if you have a different way you like to see it done we can talk about it.

Also since rebasing pretty much has to be done in order for this to make it in, figuring out how we want to handle branches would be nice. If we think we can do it without redesigning this dialog then I don't mind making that a follow up. But we are going to include it somehow on this dialog then I'd rather get it in now.

petervo · 2017-01-06T06:29:50Z

Added branches and rebasing with tests.

stefwalter

Wow that's a lot of work. Major.

We need some step by step instructions on how to test this. For the release notes we'll need to record a video, and someone will need to duplicate this.

stefwalter · 2017-01-09T07:10:41Z

pkg/lib/angular-dialog.js

+        }
+    ])
+
+    .directive('modalGroup', [


What does modalGroup do. Perhaps a multiline comment here would be helpful describing this directive.

stefwalter · 2017-01-09T07:14:17Z

pkg/ostree/app.js

@@ -161,18 +166,49 @@
                    phantom_checkpoint();
                });

-                $scope.knownVersions = function(os) {
-                    return client.known_versions_for(os);
+                $scope.switchOS = function(os) {


This function is named as if it switches the operating system in OSTree. More likely it should be named displayOS or similar, since it only changes the display.

stefwalter · 2017-01-09T07:16:22Z

pkg/ostree/app.js

+                                    branches.list = data;
+                                }, function (ex) {
+                                    branches.error = cockpit.message(ex);
+


Extra line?

stefwalter · 2017-01-09T07:17:12Z

pkg/ostree/app.js

+                                .then(function (data) {
+                                    branches.list = data;
+                                }, function (ex) {
+                                    branches.error = cockpit.message(ex);


Is it intentional that the branches.list remains at the old value in this case?

Since branches is created as a new object and only assigned to the scope after the promise is complete. branches.list will always be undefined in the error case.

stefwalter · 2017-01-09T07:22:43Z

pkg/ostree/remotes.js

+ * along with Cockpit; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+(function() {


New files don't need to use a private block. webpack puts every file in its own block.

stefwalter · 2017-01-09T07:23:09Z

pkg/ostree/remotes.js

+ */
+
+(function() {
+    "use strict";


New files don't need to use "use strict". Webpack builds all the files as strict.

stefwalter · 2017-01-09T07:27:25Z

pkg/ostree/utils.js

+ */
+
+(function() {
+    "use strict";


No need for private block or use strict in new files.

stefwalter · 2017-01-09T07:27:53Z

pkg/ostree/utils.js

+            var configRegex = {
+                section: /^\s*\[\s*([^\]]*)\s*\]\s*$/,
+                param: /^\s*([\w\.\-\_]+)\s*=\s*(.*?)\s*$/,
+                lines: /\r\n|\r|\n/


These should have comments describing what they match.

Allows us to use the dialog promise and error functionality without having to close the dialog on completion.

stefwalter · 2017-01-09T16:30:19Z

pkg/ostree/index.html

+                    <ul class="dropdown-menu">
+                        <li ng-repeat="name in os_list"
+                            ng-class="{ checked: os == name }">
+                            <a ng-click="switchOS(name)" value="{{name}}">{{name}}</a>


This needs to go back to displayOS

Support rebasing to a different remote and branch.

petervo · 2017-01-09T18:57:10Z

Updates done, easiest way to test is to use vm run with an atomic image. Use scp to put the new package in /home/admin/.local/share/cockpit. The online repos have several branches that can be used for testing.

stefwalter · 2017-01-10T10:02:57Z

This looks good to merge. But there's the following follow ups that need fixing:

" Upgrade target revision '8b15e9b988b4b02f4cb8b39bdd63d182ab7004a8926ecdac6314ee5c7ffa646b' with timestamp 'Mon Jan 9 00:43:10 2017' is chronologically older than current revision 'c2340338ffd4a0d8822e79927efed910e21d900db46c01a68b178a4284844ea3' with timestamp 'Tue Jan 10 08:27:16 2017'; use --allow-downgrade to permit "
Should use the listing pattern, as with the credentials dialog.

Support rebasing to a different remote and branch. Closes #5599 Reviewed-by: Stef Walter <stefw@redhat.com>

Closes #5599 Reviewed-by: Stef Walter <stefw@redhat.com>

petervo · 2017-01-10T14:10:49Z

This looks good to merge. But there's the following follow ups that need fixing: * " Upgrade target revision '8b15e9b988b4b02f4cb8b39bdd63d182ab7004a8926ecdac6314ee5c7ffa646b' with timestamp 'Mon Jan 9 00:43:10 2017' is chronologically older than current revision 'c2340338ffd4a0d8822e79927efed910e21d900db46c01a68b178a4284844ea3' with timestamp 'Tue Jan 10 08:27:16 2017'; use --allow-downgrade to permit "

rpm-ostreed upstream bug, fix is in and will work it's way down to all the versions eventually. Effects rebasing to older commits

* Should use the listing pattern, as with the credentials dialog.

This would probably need a redesign from @andreasn, I think he was going for keeping it similar to the tuned dialog.

andreasn · 2017-01-10T14:12:40Z

@garrett said he's happy to redesign the listing pattern.
I'll open a new issue for it.

petervo added the needswork label Dec 15, 2016

petervo force-pushed the ostree-remotes branch 6 times, most recently from f1ec493 to b781f96 Compare December 20, 2016 03:48

kubernetes: Move dialog code to lib

0cec992

petervo force-pushed the ostree-remotes branch 2 times, most recently from e27140d to 18c49d5 Compare January 6, 2017 05:19

petervo changed the title ~~WIP: Ostree remotes~~ Ostree remotes Jan 6, 2017

petervo removed the needswork label Jan 6, 2017

petervo changed the title ~~Ostree remotes~~ Ostree remotes and rebasing Jan 6, 2017

petervo force-pushed the ostree-remotes branch 3 times, most recently from 4746ef9 to db59a12 Compare January 6, 2017 22:26

stefwalter suggested changes Jan 9, 2017

View reviewed changes

stefwalter added the needswork label Jan 9, 2017

petervo added 2 commits January 9, 2017 07:43

lib: Add modal group directive

7643277

Allows us to use the dialog promise and error functionality without having to close the dialog on completion.

ostree: Manage remotes

335c929

petervo force-pushed the ostree-remotes branch from db59a12 to b0bbf11 Compare January 9, 2017 15:58

petervo removed the needswork label Jan 9, 2017

stefwalter suggested changes Jan 9, 2017

View reviewed changes

stefwalter added the needswork label Jan 9, 2017

petervo added 2 commits January 9, 2017 09:05

ostree: Rebase support

74294c9

Support rebasing to a different remote and branch.

ostree: Add Origin to info pane

6cdc164

petervo force-pushed the ostree-remotes branch from b0bbf11 to 82a8b8e Compare January 9, 2017 17:06

petervo removed the needswork label Jan 9, 2017

ostree: Remove unnecessary wrapping functions

e8e2c4e

petervo force-pushed the ostree-remotes branch from 82a8b8e to e8e2c4e Compare January 9, 2017 17:15

stefwalter approved these changes Jan 10, 2017

View reviewed changes

stefwalter closed this in 5463735 Jan 10, 2017

stefwalter pushed a commit that referenced this pull request Jan 10, 2017

ostree: Rebase support

d8fddb8

Support rebasing to a different remote and branch. Closes #5599 Reviewed-by: Stef Walter <stefw@redhat.com>

stefwalter pushed a commit that referenced this pull request Jan 10, 2017

ostree: Add Origin to info pane

c0eaeca

Closes #5599 Reviewed-by: Stef Walter <stefw@redhat.com>

stefwalter mentioned this pull request Jan 10, 2017

Various test cleanups/fixes #5701

Closed

petervo deleted the ostree-remotes branch January 10, 2017 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ostree remotes and rebasing #5599

Ostree remotes and rebasing #5599

petervo commented Dec 15, 2016

petervo commented Dec 15, 2016 •

edited

Loading

petervo commented Jan 6, 2017

stefwalter left a comment

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

petervo Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

stefwalter Jan 9, 2017

petervo commented Jan 9, 2017

stefwalter commented Jan 10, 2017

petervo commented Jan 10, 2017 via email

andreasn commented Jan 10, 2017

Ostree remotes and rebasing #5599

Ostree remotes and rebasing #5599

Conversation

petervo commented Dec 15, 2016

petervo commented Dec 15, 2016 • edited Loading

petervo commented Jan 6, 2017

stefwalter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petervo commented Jan 9, 2017

stefwalter commented Jan 10, 2017

petervo commented Jan 10, 2017 via email

andreasn commented Jan 10, 2017

petervo commented Dec 15, 2016 •

edited

Loading