meeting
LOGS
13:02:01 <mvollmer> #startmeeting meeting
13:02:01 <zodbot> Meeting started Mon Oct 24 13:02:01 2016 UTC.  The chair is mvollmer. Information about MeetBot at http://wiki.debian.org/MeetBot.
13:02:01 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
13:02:01 <zodbot> The meeting name has been set to 'meeting'
13:02:05 <mvollmer> .hello mvo
13:02:06 <zodbot> mvollmer: mvo 'Marius Vollmer' <marius.vollmer@gmail.com>
13:02:09 <dperpeet> .hello dperpeet
13:02:10 <zodbot> dperpeet: dperpeet 'None' <dperpeet@redhat.com>
13:03:01 <mvollmer> #topic Agenda
13:03:17 <dperpeet> * test images: fedora-testing, fedora-24
13:03:31 <mvollmer> * react tooltip
13:03:38 <dperpeet> * stability: cockpit.css and listing pattern
13:03:47 <mvollmer> * storage list pattern
13:04:53 <mvollmer> okay, let's go
13:05:03 <mvollmer> #topic test images: fedora-testing, fedora-24
13:05:15 <dperpeet> actually, this also concerns fedora-23
13:05:30 <dperpeet> right now we have a bunch of images that we keep track of
13:05:35 <dperpeet> but don't actually test on every pull request
13:05:51 <dperpeet> so we're back to the old question: what good does it do us to test on master?
13:05:53 <dperpeet> right now
13:06:06 <dperpeet> stef opened a pull request to remove fedora-23 from our test images: https://github.com/cockpit-project/cockpit/pull/5224
13:06:18 <dperpeet> but I think we should wait at least until fedora 25 is out (November?) to do this
13:06:25 <dperpeet> "in case we need it"
13:06:37 <dperpeet> but my opinion on this isn't very strong
13:06:45 <stefw> .hello stefw
13:06:46 <zodbot> stefw: stefw 'Stef Walter' <stefw@redhat.com>
13:06:49 <mvollmer> do we need it for "stock" testing?
13:06:59 <stefw> we have alternate stock images for testing
13:07:06 <mvollmer> i.e., testing that a f23 machine can be in a dashboard?
13:07:08 <stefw> dperpeet, i'm interested in what we need it for?
13:07:12 <mvollmer> heh, of course.
13:07:13 <stefw> mvollmer, yup
13:07:31 <petervo> fedora-testing has helped us in the past, when other packages change things and break
13:08:04 <stefw> do you use it for debugging?
13:08:07 <dperpeet> well, different reasons
13:08:10 <stefw> petervo, i'm interested in how it helped
13:08:21 <dperpeet> fedora 23: we might see if some stuff we have is backwards compatible, or run on an older distro
13:08:34 <dperpeet> or in case we do need to patch something
13:08:51 <dperpeet> end of life for fedora 23 is a little less than two months out, I think
13:09:11 <mvollmer> even if we don't test every PR with an image, and we never look at master test results, the weekly refresh of the image should give some insight
13:09:42 <dperpeet> petervo, has a good point
13:09:43 <mvollmer> that would be interesting for fedora-testing I'd say
13:09:51 <mvollmer> not so much for fedora 23
13:10:21 <dperpeet> if everyone thinks it's safe to remove fedora-23, then I don't mind much
13:10:25 <petervo> what mvollmer said, the refreshes for fedora-testing are helpful
13:10:32 <petervo> fedora-23 i don't mind removing
13:10:33 <stefw> aha, makes sense
13:10:37 <petervo> we have the stock images
13:10:38 <dperpeet> should we remove fedora-testing from master testing?
13:10:52 <dperpeet> or do we say "just keep testing"
13:10:56 <dperpeet> so we have something to look up
13:11:00 <stefw> good point about the image refreshes ...
13:11:02 <dperpeet> since it's low priority anyway
13:11:14 <stefw> my criteria would be ... if we pay attention to it ... keep it
13:11:26 <dperpeet> I haven't been paying attention to the master tests
13:11:29 <stefw> if we don't pay attention ... then it's of very dubious value
13:11:55 <mvollmer> yeah, let's kill the master tests
13:12:17 <dperpeet> ok, and what about fedora-23?
13:12:21 <dperpeet> just remove from master testing?
13:12:31 <mvollmer> remove the whole image, no?
13:12:35 <dperpeet> ok
13:12:41 <dperpeet> and keep fedora-testing
13:12:46 <dperpeet> for debugging and if other stuff breaks
13:12:53 <dperpeet> but not test it on master
13:12:55 <mvollmer> i would say so
13:12:59 <dperpeet> and fedora-24?
13:13:04 <dperpeet> also remove master testing
13:13:05 <dperpeet> ?
13:13:12 <mvollmer> i wouldn't mind
13:13:16 <dperpeet> basically test it once an image refresh
13:13:27 <dperpeet> that basically maps to what we do anyway
13:13:34 <dperpeet> look at the test when it's being refreshed
13:13:48 <dperpeet> ok, I'll update my fedora-testing pr to reflect this
13:14:41 <mvollmer> okay
13:14:46 <mvollmer> next?
13:15:05 <dperpeet> yup
13:15:13 <mvollmer> #topic react tooltip
13:15:30 <mvollmer> I have made a react tooltip widget thing as part of https://github.com/cockpit-project/cockpit/pull/5097
13:15:40 <mvollmer> the storage list pattern rewrite
13:15:52 <mvollmer> I have rewritten it three times or so
13:16:02 <stefw> is there a design pattern page?
13:16:21 <mvollmer> it is part of the react pattern playground demo
13:16:33 <mvollmer> I could split that out as a separate PR
13:16:58 <mvollmer> but it's also good to actually use it and review it in action as part of #5097
13:17:01 <dperpeet> note on this: I took mvollmer's listing pattern changes and made a new pr with some fixes https://github.com/cockpit-project/cockpit/pull/5223
13:17:05 <stefw> mvollmer, yup
13:17:17 <dperpeet> so that should make #5097 a little smaller also
13:17:38 <mvollmer> so, should I leave the tooltip in #5097?
13:18:04 <dperpeet> how soon do you think this can go in?
13:18:31 <mvollmer> well, my TODO list is empty...
13:18:42 <mvollmer> but there will be lots of stuff from the reviws
13:19:06 <mvollmer> so a few weeks still, I guess
13:19:14 <dperpeet> I would prefer to have a separate PR for the tooltip
13:19:17 <mvollmer> if we have another user for the tooltip, then I can split it out
13:19:33 <dperpeet> I will need it for the docker run stuff
13:19:37 <mvollmer> yeah
13:20:05 <mvollmer> it doesn't have to be perfect
13:20:23 <dperpeet> if we have a separate PR, it's easier to look up discussions later on
13:20:28 <mvollmer> what I like is that we can make it behave exactly like we want
13:20:34 <mvollmer> or like patternfly wants
13:21:05 <mvollmer> i find the bootstrap tooltip slightly magic
13:21:17 <dperpeet> yes, I agree
13:21:24 <mvollmer> but on the other hand
13:21:27 <dperpeet> let's make a separate PR that we can tinker with
13:21:29 <dperpeet> in the playground
13:21:36 <mvollmer> bootstrap has figured out all the compat issues
13:21:55 <mvollmer> on first try it didn't work at all in chrome, for example
13:21:57 <dperpeet> worries about reinventing the wheel?
13:21:58 <mvollmer> but yeah
13:22:22 <mvollmer> (because disabled buttons don't fire mouseenter events, I think.
13:22:28 <mvollmer> stuff like that
13:22:40 <dperpeet> react bootstrap is sadly not a good reference
13:23:01 <mvollmer> there is react bootstrap?
13:23:06 * mvollmer googles
13:23:08 <dperpeet> https://react-bootstrap.github.io/
13:23:12 <dperpeet> pre-1.0
13:23:28 <dperpeet> I looked into that while working on the dialog template
13:23:55 <dperpeet> https://react-bootstrap.github.io/components.html#tooltips specifically
13:24:46 <dperpeet> We probably want https://react-bootstrap.github.io/components.html#popovers oftentimes
13:25:38 <mvollmer> hmm, I have to check this more closely.
13:25:49 <dperpeet> mvollmer, let's talk about this some other time
13:25:54 <mvollmer> yes
13:26:00 <dperpeet> regarding the topic
13:26:06 <dperpeet> I think this deserves its own pull request
13:26:12 <dperpeet> since we want to do it right
13:27:56 <mvollmer> yes
13:28:42 <mvollmer> #topic  stability: cockpit.css and listing pattern
13:29:43 <dperpeet> as mentioned above, mvollmer and I added some changes to use the react listing pattern in cases where we don't want rows to expand
13:29:44 <dperpeet> https://github.com/cockpit-project/cockpit/pull/5223
13:30:02 <mvollmer> this is used in the storage content
13:30:10 <dperpeet> this also brought home that we have a bunch of listing pattern css in cockpit.css
13:30:13 <mvollmer> but only for weird rows that represent "free space"
13:30:24 <mvollmer> maybe we won't have them in the end actually
13:30:29 <dperpeet> and if we touch that, I think we could break stability pretty quickly
13:30:38 <dperpeet> when mixing cockpit modules of different versions
13:31:11 <dperpeet> mvollmer, "free space rows" are a different issue, I'd say
13:31:29 <dperpeet> the question is, how stable should cockpit.css be?
13:31:45 <stefw> is there any way we can, with good conscience, get the listing pattern out of cockpit.css
13:31:48 <dperpeet> when would we want to migrate the relevant css into lib?
13:31:54 <petervo> doesn't each module load it's own cockpit.css
13:32:11 <petervo> i mean each machine
13:32:15 <petervo> loads it's own
13:32:23 <petervo> there shouldn't be mixing between machines
13:32:34 <dperpeet> yeah, but what if you have cockpit-base 0.117 and cockpit-storage 123?
13:32:52 <dperpeet> then storage would expect the newer css
13:33:04 <larsu> storage would bundle the newer css in that case, no?
13:33:34 <petervo> or the package would require the newer one if it's breaks without it
13:33:54 <petervo> same as if it needs a newer cockpit.js feature
13:34:14 <larsu> ah, indeed
13:34:39 <mvollmer> but one could argue that the listing CSS is only internal
13:34:39 <petervo> but yes i'm ok with moving listing out of cockpit.css as well
13:34:51 <dperpeet> I think this is worth breaking compatibility
13:34:55 <mvollmer> private to cockpit-component-listing.jsx
13:34:59 <dperpeet> for the sake of cleaning this up
13:35:12 <petervo> listing is also used in angular code
13:35:15 <dperpeet> petervo, yes, I think we'd need to require the right version
13:35:16 <dperpeet> yes
13:35:36 <mvollmer> petervo, right
13:35:55 <dperpeet> I can think about how this would work out in practice
13:36:12 <dperpeet> if we're ok with changing the require clauses in general
13:40:50 <dperpeet> in any case, we don't want to create extra work and we don't want to break stuff that's using the listing pattern
13:42:55 <mvollmer> so, you think the fix is generally a good one?
13:43:03 <mvollmer> supporting rows without any tabs in them?
13:43:42 <dperpeet> I think so
13:43:48 <dperpeet> we don't always want to expand
13:44:07 <dperpeet> that way we still keep the same css
13:44:09 <mvollmer> okay
13:44:12 <dperpeet> larsu, what do you think?
13:44:41 <larsu> yeah, I agree as well
13:45:21 <mvollmer> larsu, can you do the review?
13:45:29 <stefw> are we agreeing to break backward compatibility by moving listing out of cockpit.css?
13:46:10 <dperpeet> and looking at the other stuff in there
13:46:12 <larsu> stefw: we can't move it out completely, because it's being used by things that are not the react listing component
13:46:27 <stefw> well the CSS could definitely be moved into lib/
13:46:36 <stefw> and the other stuff could then just include it in their own CSS
13:46:42 <dperpeet> exactly
13:46:44 <stefw> the only issue would be compatibility
13:46:47 <larsu> mvollmer: sure! 5223?
13:46:54 <mvollmer> yes
13:47:14 <stefw> and if we assume nobody is supposed to be using this anyway, was never supposed to be there ... then we may be able to in good conscience remove it.
13:47:33 <stefw> bit of a gray area for sure
13:49:29 <dperpeet> ok, I'll look at the consequences and open a pr
13:50:02 <mvollmer> next?
13:51:48 <mvollmer> #topic  storage list pattern
13:52:05 <mvollmer> this is mostly for andreas, I guess
13:52:20 <mvollmer> I think it is getting ready for serious UX review
13:52:25 <mvollmer> maybe not yet the code
13:52:42 <mvollmer> https://github.com/cockpit-project/cockpit/pull/5097
13:52:53 <mvollmer> I think it worked out nice
13:53:00 <mvollmer> I ade some follow up notes in trello
13:53:17 <mvollmer> and have a list of smaller fixes
13:54:11 <mvollmer> i guess I make a screencast for this
13:54:41 <dperpeet> sounds good
13:55:23 <mvollmer> I'll hopefully get to the new features that this should enable pretty soon
13:55:31 <mvollmer> but the change is good all by itself
13:55:45 <mvollmer> maybe I'll rework networking first to use the list pattern?
13:56:31 <dperpeet> if that brings an actual improvement, then yes
13:56:38 <dperpeet> but not just for the sake of rewriting things
13:58:51 <mvollmer> well, to get the "standard look & feel"
13:59:44 <larsu> I don't think that's enough of an argument :)
13:59:51 <mvollmer> yeah
13:59:57 <larsu> style comes and goes and will probably change all the time
14:00:38 <mvollmer> alright
14:00:49 <mvollmer> #topic any other business
14:01:15 <mvollmer> https://plus.google.com/photos/photo/109789677627554448061/6344293315971779970?icm=false
14:01:31 <larsu> haha
14:01:44 <larsu> (google+ is still a thing?)
14:02:18 <mvollmer> yes, looks like it
14:02:40 <mvollmer> systemd is medium active there
14:02:40 <stefw> heh
14:02:50 <stefw> puns make me cry
14:02:55 <github> [cockpit] dperpeet reopened pull request #5218: Image refresh for fedora-testing (master...refresh-fedora-testing-2016-10-22) https://git.io/vPFuq
14:04:03 <mvollmer> okay, thanks everyone
14:04:06 <mvollmer> #endmeeting