2020-04-23 UTC
kino joined the channel
[jgmac1106] joined the channel
# 00:26 gRegorLove So once this is in master, we could bump php-mf2 to 0.5.0 for the next release instead of putting it behind a flag # 00:27 gRegorLove Though we're still in that special zero major version, where anything can change at any time and API shouldn't be considered stable (according to semver) [Kevin_Faaborg], [fluffy], [chrisaldrich], beko, chimo, KartikPrabhu and Sajesajama joined the channel
# 03:36 Loqi [Zegnat] #14 Should the specification imply properties for <link> elements? KarlieKloss, [Aaron_Klemm], alexmingoia[dot], alexmingoia-dot-, alexmingoia, [KevinMarks], KartikPrabhu, [LewisCowles], [chrisaldrich], [tw2113], [mapkyca] and itsme1 joined the channel
# 09:22 Loqi [dshanske] #218 Parse an image for src and alt [jgmac1106], Feliciana, Sajesajama, KartikPrabhu, mauz555 and [mapkyca] joined the channel
# 11:31 Loqi Zegnat has 4 karma in this channel over the last year (64 in all channels) [jeremycherfas] and [jgmac1106] joined the channel
# 12:38 Zegnat GWG is it OK if I try to commit that patch I shared yesterday to your feature branch? I am not sure GitHub will let me, but I would like to try. But don’t want to get in your way if you’d rather commit all code yourself # 12:39 GWG Zegnat: I'm fine if you want to. I was going to do it last night, but I was in the HWC West Coast # 12:43 Zegnat I never actually tried pushing straight to someone else’s branch before KartikPrabhu, itsme1 and [grantcodes] joined the channel
# 12:55 GWG pfefferle does it to me sometimes # 12:56 GWG Also, he apparently pointed out I'm using WP coding style # 12:57 Zegnat I don’t think we have a coding style on mf2 parser? Is a bit all over the place # 12:58 Zegnat I think there are both tabs and spaces for indent throughout those files? Maybe we should set a code style at some point, but I am not too bothered. # 12:59 Zegnat I pushed a series of 4 minimal commits now, GWG. Hopefully all very self-explanatory. # 12:59 Zegnat Added a breaking test for the Sally Ride implied h-card. Then a commit that fixes it. Then a commit that brings another test inline with the new code. And finally a commit where I rewrite a broken test to actually be 3 different tests. # 12:59 Zegnat 3 different tests so we can test for no alt, empty alt, and an alt with content. # 13:07 Zegnat And Travis is done, all tests are green! Nice work on that change, GWG! # 13:07 Loqi GWG has 4 karma in this channel over the last year (139 in all channels) # 13:08 Zegnat Please have a look at the pushes I did, maybe you will realise there is something more that needs to have tests created for it :) [jgarber] joined the channel
# 13:27 [jgarber] Zegnat: GitHub’s struggling again. What’s the change in the PR? # 13:28 Zegnat [jgarber]: In GWG’s PR? Mostly updating and adding tests to support img alt parsing # 13:29 [jgarber] Zegnat: Gotcha. Thanks! What are the implications for microformats/tests? # 13:30 Zegnat Do not know yet. But I do not think microformats/tests was updated with support for alt parsing at all yet? Or was it? # 13:31 Loqi [jansauer] #109 Reflect the parsing rules for src and alt attributes on img elements # 13:33 Zegnat I don’t know. The PHP parser is not actually tested against microformats/tests so it is hard to know sometimes. # 13:33 Zegnat That last PR I linked is to get the PHP parser using microformats/tests, but last time I tried there were lots of incompatibilities. # 13:36 Zegnat Why did GitHub have to have troubles right now when I am having a bit of a break from work and want to do some mf stuff?! # 13:36 [jgarber] sknebel: With a little nudging this week, I was able to get a handful of microformats/tests PRs closed out. 😄 # 13:36 [jgarber] Zegnat: I know, right? Rough week for the GitHub operations crew. # 13:38 Zegnat Tests: 383, Assertions: 849, Failures: 44, Skipped: 1. - when I run my microformats/tests in the PHP parser # 13:38 Zegnat Tests: 315, Assertions: 787, Skipped: 1. - when I run GWG’s branch with the img alt parsing # 13:39 Zegnat So there is why the tests are not yet run as part of the PHP parser ;) # 13:39 Zegnat Last time I rebased the branch was Aug 24, 2018 # 13:39 sknebel [jgarber]: yeah, but I had totally thought those had been already handled, otherwise I'd done it earlier :/ # 13:40 [jgarber] sknebel: Not to worry! I only noticed them since I’ve been using the test suite against some code I’m writing. It got me interested and thinking about microformats/tests. # 13:42 [jgarber] Feels like it’s _supposed_ to be the canonical authoritative test suite, but it’s also not well-used by parser code bases (maybe?). Or, at least, parser code bases are very out-of-date against the repo. # 13:42 Zegnat Aah, still seeing lots of problems with dates # 13:45 Zegnat Hmm, I should log more of the exact test file and not just the name. Harder to find it in the tests repo now # 13:46 Zegnat Hmm, looks right in the repo :/ Maybe something else whacky on my end [LewisCowles] joined the channel
# 13:58 Zegnat Woop, I thought I was fixing how to display test names, and in the process I apparently got it to run (and fail) more tests :D # 13:58 Zegnat Tests: 432, Assertions: 898, Failures: 68, Skipped: 1. [KevinMarks] joined the channel
# 14:07 Zegnat [jgarber]: example test fail, microformats-v1/hfeed/simple. Seems to be because it expects to magically add example.com? # 14:07 Zegnat Do the tests expect all files to be parsed as if fetched from example.com? # 14:27 [jgarber] Zegnat: That appears to be the assumption, yeah. Not sure if that’s documented anywhere, though. # 14:33 [jgarber] Zegnat: I’m about fifteen years out of practice on the PHP front… 😂 # 14:34 Zegnat Ah, well, apart from having to run composer install, not a lot of changes in those last fifteen years ;) # 14:34 Zegnat Waiting for Travis to run the tests. That might give a nice browsable overview of what needs fixing # 14:34 GWG I see a lot of things going on as I work # 14:35 Zegnat is working from home today and has an easier time dropping in and out of IRC Sajesajama_ joined the channel
# 14:45 [jgarber] Zegnat: If I’m reading that failure right, the PHP parser is _not_ returning `name` ? # 14:46 Zegnat As in, you are correct, PHP is not returning it but it is in the expected output from the test suite # 14:48 Zegnat Although, hmm, that may still be a PHP parser bug # 14:48 [jgarber] Right. And I think the test is demonstrating implied name in the scope of `h-item` (not `h-review`) # 14:48 Zegnat The h-item has no properties defined at all, not sure why it thinks otherwise # 14:49 [jgarber] Right. The implied name is for the nested `h-item`, not the top-level `h-review` (where no name is implied in the output). # 14:49 [jgarber] No name is implied on the `h-review` because of the rules you noted above. # 14:49 Zegnat So yeah, now that we can run this again, it is time to go through all failures one by one # 14:49 [jgarber] So yeah, the test is demonstrating several things at once. # 14:50 Zegnat The worst are the ones like the third failure, where you have to be very careful in checking why an e- property was not parsed in the same way # 14:50 [jgarber] 3. Imply name when neither of the above two conditions are true Sajesajama_ joined the channel
# 14:53 Zegnat And also spacing within the plain text `value` is removed in the PHP parser but not in the test suite # 14:54 [jgarber] Error 11 looks to be related to sorting types arrays alphabetically. # 14:54 [jgarber] > `type: [array of unique microformat "h-*" type(s) on the element sorted alphabetically],` # 14:55 Zegnat Yep, so that definitely needs a PR in the test suite # 14:55 [jgarber] Re: e- parsing `value` … I think only leadiing/trailing space is to be removed. # 14:55 Zegnat I think I may look into this omission of implied name, that looks much more like a parser bug, and may get the number of total test failures down # 14:56 Zegnat is pretty sure we implemented sorting in the parser # 14:57 Zegnat // Make sure things are unique and in alphabetical order [tw2113] joined the channel
# 14:58 [jgarber] Error 16 seems to refer to specs that aren’t in microformats/tests… “microformats-v2/h-news/all” # 14:58 Zegnat According to the test output there on Travis it is the test suite that was expecting the wrong order. # 14:59 Zegnat Can Travis have grabbed some older version of the tests? # 14:59 [jgarber] (if that’s how the PHP parser is pulling microformats/tests) # 15:02 Zegnat Must be the lock file. Since the test suite isn’t versioned, we just point at master # 15:04 Zegnat But even just the update to 92b5893 gets us down to 40 failures! # 15:07 Zegnat And only the first 11 are in mf2 tests, the others are in mf1 backcompat. And I know there are a few still open PRs from gRegorLove to fix some backcompat parsing # 15:11 Loqi [Zegnat] php-innertext: 🏃🐉 Run. Here be dragons. # 15:34 Zegnat sknebel: everytime I am reminded of that innerText implementation I get flashbacks to the beach chairs at Tollwerk ^^^ :D # 16:03 Zegnat Alright, waiting for Travis to kick at it again. Now hard linked to the latest commit on the github repo # 16:04 Loqi Ok, I'll tell them that when I see them next Sajesajama joined the channel
# 16:39 [jgarber] Looks like whitespace is the primary cause for many of the remaining failures, yeah? # 16:42 Zegnat Yeah, wish I had a way of ignoring those, but I do not think I can gRegorLove joined the channel
# 16:53 Zegnat The problem is that I am comparing 2 JSON structures. And sometimes I want to ignore whitespace differences between 2 specific values. [mapkyca] joined the channel
# 16:54 Zegnat Alternatively I add a flag to the parser that lets you get the old behaviour again. # 16:55 Zegnat But didn’t the Python implementation also implement the newer whitespace behaviour? Maybe instead of patching how tests are run, we need to look at whether we want to move it forward in the parsing spec # 17:02 GWG Zegnat: I am saying we may need a custom Assert function then # 17:03 Zegnat Wouldn’t help because I do not know what items in the JSON output need to use it # 17:04 Zegnat I guess I could do it for e-* properties, as those are always value properties next to html properties. And then just strip whitespace from both. # 17:05 Zegnat But does that guarantee that the test was supposed to pass in the first place? Unsure. # 17:07 Zegnat Oh definitely! So now it is interesting to just manually check the things that go wrong. And either fix the test suite or fix the PHP parser. # 17:09 GWG I'm looking forward to getting alt text in. # 17:09 GWG I think after that settles, srcset jamietanna joined the channel
# 17:12 Loqi [tantek] #7 Should u-* parsing special case img srcset? [Aaron_Klemm] and [tantek] joined the channel
# 17:48 GWG gRegorLove: Is this PR mergable now? [snarfed] joined the channel
# 17:51 Loqi [gRegorLove] #188 Specify coding standard Zegnat and sknebel joined the channel
# 18:02 [tantek] I have a boring GitHub pull request review for you gRegorLove when you have a moment (timely would be good) jamietanna, [mapkyca], vesper and [LewisCowles] joined the channel
# 19:59 Zegnat I guess when we get to the point that all of them pass, we can simply take away the exclude line and we’ll be covered! # 20:23 Zegnat [tantek]: no, I did not add more granularity than splitting per folder. # 20:24 Zegnat If you have any ideas on how you’d want to manage that, please leave a comment :) # 20:25 [tantek] Zegnat, no idea, just worried about having zero tests run by default # 20:26 Zegnat Default when I run PHPUnit in that branch: Tests: 311, Assertions: 777, Skipped: 1. [chrisaldrich] joined the channel
# 21:27 GWG Okay. What else is left for a new major release? [Aaron_Klemm] joined the channel
# 22:02 gRegorLove Hasn't been reviewed in a while, so should check if we want to get any others in # 22:03 gRegorLove I've done some initial checking on 205 and might need to defer on it; could be a complex fix # 22:03 gRegorLove I think we also need to discuss if next release should be 0.5.0 instead of 0.4.7. img+alt is breaking change # 22:04 gRegorLove But maybe that doesn't matter since semver says anything can change < 1.0 # 22:17 gRegorLove I think we jumped to 0.4 last time there was a breaking change # 22:23 beko Breaking change on Patch are bad. Nobody expects this no matter the Major. People look closer if Minor changes.
gRegorLove Makes sense
Zegnat Slept a solid right hours and now have the headspace to think about stuff again! I'll have a look at https://github.com/microformats/php-mf2/pull/218 again on my lunch break GWG. Let's see if we can get it merged today-ish!
[jgarber] This PR was merged this week. I think it may be related: https://github.com/microformats/tests/pull/109
KartikPrabhu it'd be cool if the test suite passes php-mf2 ;)
