| 08:17 | <tobie> | Is there a trick to close a PR when using the command line to modify a commit and push it to upstream directly? |
| 08:18 | <tobie> | I just tried that with https://github.com/heycam/webidl/pull/165 and failed. |
| 08:18 | <annevk> | tobie: you modify the commit on the branch, force push, then merge it using fast forward |
| 08:19 | <annevk> | tobie: https://github.com/whatwg/html/blob/master/TEAM.md has various pointers |
| 08:19 | <tobie> | annevk: thanks for the resource. |
| 08:19 | <annevk> | tobie: https://github.com/whatwg/html/blob/master/TEAM.md#merging-pull-requests-from-branches in particular |
| 08:20 | <tobie> | annevk: that implies having push rights to the branch, though, right? |
| 08:24 | <annevk> | tobie: yeah, if you don't the only way to close the PR is to add close #{prnumber} to the commit message, but that's somewhat frowned upon for reasons not entirely clear to me |
| 08:24 | <tobie> | annevk: because it closes the PR but the PR appears to not have been merged. |
| 08:25 | <annevk> | Sure, but assuming you cannot merge the PR successfully (external branch that requires a change other than a commit message change) that's kinda your only option |
| 08:26 | <tobie> | yup |
| 08:26 | <tobie> | oh well. |
| 08:31 | <annevk> | tobie: you should prolly give Domenic write access to that repo so he can create branches |
| 08:31 | <annevk> | tobie: and then protect master to only give the editors push access to that branch |
| 08:31 | <annevk> | tobie: or some such |
| 08:32 | <annevk> | tobie: I think that's the way to go if you have contributors that come back a lot, just give them write access so you can collaborate on branches, but protect master from accidents (other than your own) |
| 08:32 | <tobie> | annevk: I don't have admin access. |
| 08:32 | <tobie> | annevk: I guess heycam does |
| 08:32 | <annevk> | heycam: ^^ |
| 08:32 | <annevk> | would be weird if not |
| 08:35 | <heycam> | how do you do fine grained access control like that? |
| 08:35 | <heycam> | branch protection for gh-pages says "Disables force-pushes to this branch and prevents it from being deleted" so I guess that's not it |
| 08:39 | <annevk> | heycam: should be at https://github.com/heycam/webidl/settings/branches/master |
| 08:39 | <annevk> | heycam: or https://github.com/heycam/webidl/settings/branches/gh-pages I guess |
| 08:39 | <annevk> | heycam: should have "Restrict who can push to this branch" |
| 08:39 | <heycam> | annevk: no I don't see that |
| 08:39 | <heycam> | maybe that's only available for orgs? |
| 08:40 | <annevk> | heycam: https://github.com/blog/2051-protected-branches-and-required-status-checks none of this is there? |
| 08:41 | <heycam> | annevk: no I see that |
| 08:41 | <annevk> | heycam: so maybe you first need to make gh-pages a protected branch |
| 08:41 | <annevk> | heycam: and then once it is, you can add further protections |
| 08:42 | <heycam> | I don't see any more options even after I enable protection |
| 08:42 | <heycam> | just the "Require status checks " one |
| 08:42 | <heycam> | oh that opens some more things when I check it |
| 08:42 | <heycam> | grr |
| 08:43 | <heycam> | though I'm not sure if the status checks one is what I want either... |
| 08:43 | <annevk> | heycam: yeah, status checks isn't important unless you have some integration thing setup |
| 08:44 | <heycam> | so I just see this: http://mcc.id.au/temp/branch.png |
| 08:45 | <annevk> | heycam: fascinating |
| 08:46 | <annevk> | heycam: maybe it's only available for "master" |
| 08:46 | <heycam> | hmm |
| 08:46 | <annevk> | heycam: no, it's an org feature as you thought |
| 08:46 | <heycam> | ah |
| 08:46 | <heycam> | how about I just add domenic and he can exercise restraint, in only pushing branches |
| 08:46 | <annevk> | https://help.github.com/articles/about-protected-branches/ |
| 08:46 | <annevk> | yeah |
| 08:47 | <annevk> | do that |
| 08:47 | <heycam> | ok done |
| 09:15 | <Ms2ger> | annevk, tobie: btw, there's also https://github.com/blog/2247-improving-collaboration-with-forks now |
| 09:17 | <annevk> | Ms2ger: oh wow, that's great |
| 09:27 | <MikeSmith> | yeah very nice |
| 09:32 | <tobie> | Ms2ger: oh, I thought I had seen that somewhere! Thanks for sharing. |
| 14:49 | <tobie> | bz: sorry you're having issues building Bikeshed |
| 14:49 | <tobie> | bz: TabAtkins should be able to help you debug. |
| 15:34 | <JakeA> | annevk: when/why did headers.getAll vanish? |
| 15:48 | <JakeA> | nm found https://github.com/whatwg/fetch/commit/42464c |
| 15:54 | <JakeA> | annevk: new Headers([['foo', 'bar'], ['foo', 'yay']]).get('foo') returns "bar" in both Firefox & Chrome. It should be "bar,yay" right? |
| 15:54 | <annevk> | JakeA: yeah |
| 15:54 | <JakeA> | filing |
| 15:55 | <annevk> | JakeA: WebKit made a convincing argument that since we don't expose cookies, we shouldn't expose non-mapness to developers |
| 15:56 | <JakeA> | annevk: should they be combined at iteration time too? |
| 15:56 | <JakeA> | [...new Headers([['foo', 'bar'], ['foo', 'yay']])].length == 2 in Chrome |
| 15:57 | <annevk> | JakeA: yes |
| 15:57 | <annevk> | JakeA: iteration is the result of https://fetch.spec.whatwg.org/#concept-header-list-sort-and-combine |
| 15:58 | <JakeA> | ta, will file |
| 16:06 | <JakeA> | annevk: Safari combines with ', ' rather than ',' - are they wrong? |
| 16:10 | <annevk> | JakeA: technically, I think they're fixing that |
| 16:10 | <JakeA> | cool |
| 16:11 | <JakeA> | annevk: https://bugzilla.mozilla.org/show_bug.cgi?id=1301713 |
| 16:12 | <annevk> | ta |
| 16:14 | <JakeA> | annevk: and https://bugzilla.mozilla.org/show_bug.cgi?id=1301714 |
| 16:16 | <annevk> | JakeA: you might care about https://github.com/WICG/async-cookies-api/issues/36 |
| 16:20 | <JakeA> | I am, cheers! |
| 16:20 | <JakeA> | am interested that is |
| 16:26 | <bkardell> | dglazkov u around? |
| 16:27 | <bkardell> | actually... anybody around who might have more discussion history on this https://docs.google.com/document/d/1E16tmoOveGopys9Fx4Nn8x8P9q7Jm6CJIqoJyz31tlg/edit |
| 16:35 | <TabAtkins> | bkardell: What's up? |
| 16:36 | <bkardell> | do you have additional links on where aspects of this were discussed? |
| 16:37 | <TabAtkins> | No, tho there is some links in the doc, and several issues in the wicg repo you can look to for more details. |
| 16:37 | <bkardell> | like... I have concerns/thoughts but I'm sure it's pretty likely they've been discussed |
| 16:37 | <TabAtkins> | What's the context of your question? What specifically do you want to know? |
| 16:37 | <bkardell> | yeah I've been looking |
| 16:43 | <tobie> | Domenic: Rebased https://github.com/heycam/webidl/pull/170, think you could give it a look? |
| 16:52 | <annevk> | TabAtkins: see https://github.com/whatwg/dom/pull/313#issuecomment-242428180 for information about what might cause the span noise |
| 16:53 | <TabAtkins> | Oh, thanks! |
| 16:53 | <TabAtkins> | Hmm. |
| 17:07 | <annevk> | My Pygments version is 2.0.2 |
| 17:07 | <annevk> | Was there some regression inbetween? |
| 17:08 | <Domenic> | Yes, I think newer versions add empty spans |
| 17:08 | <annevk> | Bah |
| 17:11 | <Domenic> | Maybe Bikeshed should do some kind of version check on pygments before running |
| 17:18 | <Domenic> | TabAtkins: in HTML, do I need to do `<dfn data-lt="transferable object">transferable objects</dfn>` or will Bikeshed/Shepherd figure out the pluralness itself? |
| 17:25 | <TabAtkins> | Domenic: It's best to lt the "base" version. Bikeshed will handle <dfn>conjugated</dfn> => <a>conjugate</a>, but not <a>conjugating</a>. (It only goes to/from the base form once.) |
| 17:26 | <TabAtkins> | I didn't know there were empty spans in newer versions. Should be trivial to remove manually. |
| 17:27 | <TabAtkins> | Hm, looks like they're always just at the start of the highlight blocks? |
| 17:55 | <TabAtkins> | and fixed |
| 18:34 | <jyasskin> | TabAtkins: https://github.com/tabatkins/bikeshed/commit/127d0ac944ee152012efcc16c66ed88b50177810 broke the permissions spec. Any suggestions for how to let other specs define the enum-values from https://w3c.github.io/permissions/#enumdef-permissionname? |
| 18:51 | <jyasskin> | tobie Domenic: ^ |
| 18:58 | <jyasskin> | I can define a bunch of new terms like `"bluetooth" permission-related algorithms`, and have a lookup table in the Permissions spec. We might have to do that anyway once PermissionName merges into the Feature Policy spec. |
| 19:19 | <TabAtkins> | jyasskin: If Permissions isn't defining the values, it shouldn't have them in WebIDL blocks, as those are inferred to be defining. (I'm really not sure what I was thinking with my original "link elsewhere if already defined" code in the first place; I think maybe I just didn't know about partial?) If you want it to look WebIDL-ish, just use <pre |
| 19:19 | <TabAtkins> | class=lang-webidl>, and write <a>s in the source to link them up. |
| 19:20 | <TabAtkins> | (Sorry for breaking your previous use; you didn't know it was non-idiomatic, and that was my fault.) |
| 20:32 | <tobie> | jyasskin: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27048 |
| 20:33 | <tobie> | jyasskin: and in particular: https://www.w3.org/Bugs/Public/show_bug.cgi?id=27048#c3 |
| 20:34 | <tobie> | jyasskin: but there seems to be a lot of resistance to that idea. |
| 20:40 | <jyasskin> | tobie: Yeah, it's important that there be an index that points to all the enum values, although Shepherd might give us that soon. Faking the IDL block with explicit links might be the way to go here, until the next set of things changes. |
| 21:01 | <jyasskin> | TabAtkins: Using class=lang-webidl makes the enum disappear from the IDL Index. |
| 21:02 | <TabAtkins> | Yes, because then you're not defining it. ^_^ Okay, let's start over from the beginning. You want to define an enum, but not define any of its values? Or maybe just some of its values, and let other specs define other values, but still document those external values in this core spec? |
| 21:02 | <jyasskin> | Exactly. |
| 21:02 | <jyasskin> | It's a registry, where remote specs ought to define their set of algorithms and types, and then the Permissions spec should link to them. |
| 21:03 | <jyasskin> | Initially, most of the entries are defined locally, but it doesn't work well for the Permissions editors to maintain the details of, say, the Media algorithms. |
| 21:03 | <TabAtkins> | Okay. So. In a normal WebIDL block, put the enum and the values that you define in this spec. Then gimme a sec to fix up the (purposely undocumented) <index> element so it'll work for your use-case; it's 95% of the way there right now. It'll generate an index of all known enum-value values for the given enum. |
| 21:05 | <jyasskin> | !!! So then other specs can write, what, `enum PermissionName { "my-value" }`, and the <index> will automatically link to there? |
| 21:05 | <jyasskin> | Where would {{PermissionName}} link in that case? We really do want tobie's suggestion of partial enums for this. |
| 21:06 | <TabAtkins> | Well, we dont' have partial enum support in WebIDL, so they'll have to do it manually, with <dfn enum-value for=PerimissionName>"my-value"</dfn>, but yes. |
| 21:06 | <jyasskin> | Which is what they're doing right now, so that's fine. |
| 21:06 | <TabAtkins> | If they did it with WebIDL, then {{PermissionName}} would just complain about multiple possible refs. |
| 21:07 | <jyasskin> | Will the <index> add the other values to the IDL Index in the Permissions spec? |
| 21:08 | <TabAtkins> | No, it'll generate an index right there, like in https://drafts.csswg.org/indexes/ |
| 21:08 | <TabAtkins> | So you'd need some explanatory text around it to give it context. |
| 21:09 | <jyasskin> | Mhmm. And we'd lose the unified IDL for PermissionName that implementers can copy into their .idl files, but that's not the end of the world. |
| 21:10 | <TabAtkins> | I could work on something to make that work a little better; I see the value in having a "unified" IDL block for anything that can be partial'd. |
| 21:10 | <TabAtkins> | But the index thing is nearly ready right now. ^_^ |
| 21:10 | <Domenic> | We should probably add partial enums |
| 21:13 | <jyasskin> | Yeah, the index is probably better than what I'm doing right now. |
| 21:15 | <TabAtkins> | It'll be automagical, so yeah. |
| 21:23 | <tobie> | jyasskin: in that case, isn't the unified block actually *not* what you want? As I mentioned in the comment linked above, why would you want an enum value representing a permission for something you haven't implemented? |
| 21:25 | <jyasskin> | tobie: For Bluetooth, we take the IDL blocks and comment out the lines we haven't implemented yet: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.idl?l=17. But maybe people won't actually copy the PermissionName IDL block verbatim into their implementations. |
| 21:28 | <tobie> | jyasskin: |
| 21:29 | <tobie> | (sorry, auto tab mixup) |
| 21:38 | <tobie> | jyasskin: from an implementor's perspective, I guess this boils down to whether if fits the code structure more to group the permissions together, or scatter them across the different features they enable. I would imagine the former be easier to setup and the latter to maintain, by ymmv. |
| 21:40 | <jyasskin> | Yeah. I don't know for sure whether it'd help folks to have an IDL Index that includes everything. |
| 21:47 | <jyasskin> | Domenic/annevk: I'd like to use "relevant settings object" without an explicit platform object to refer to the relevant settings object of the *context object*. Does that sound like a good idea? I'll send you an HTML patch if so. |
| 21:47 | <jyasskin> | I could also just write "this's relevant settings object". |
| 22:14 | <Domenic> | jyasskin: yeah seems bad. This's or the context object's seems better. |
| 22:17 | <jyasskin> | Domenic: 'k |
