{"action":"created","issue":{"url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/1","repository_url":"https://api.github.com/repos/ndrake0027/drupaljunitparser","labels_url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/1/labels{/name}","comments_url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/1/comments","events_url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/1/events","html_url":"https://github.com/ndrake0027/drupaljunitparser/issues/1","id":316995609,"number":1,"title":"Misc cleanups","user":{"login":"kevin-dutra","id":669424,"avatar_url":"https://avatars1.githubusercontent.com/u/669424?v=4","gravatar_id":"","url":"https://api.github.com/users/kevin-dutra","html_url":"https://github.com/kevin-dutra","followers_url":"https://api.github.com/users/kevin-dutra/followers","following_url":"https://api.github.com/users/kevin-dutra/following{/other_user}","gists_url":"https://api.github.com/users/kevin-dutra/gists{/gist_id}","starred_url":"https://api.github.com/users/kevin-dutra/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kevin-dutra/subscriptions","organizations_url":"https://api.github.com/users/kevin-dutra/orgs","repos_url":"https://api.github.com/users/kevin-dutra/repos","events_url":"https://api.github.com/users/kevin-dutra/events{/privacy}","received_events_url":"https://api.github.com/users/kevin-dutra/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-04-23T21:47:24Z","updated_at":"2018-05-01T21:59:30Z","closed_at":null,"author_association":"NONE","body":"https://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L9\r\n\r\nThe namespacing seems a little odd. I'd either expect this to be either a single top level item that describes the project (e.g. `DrupalJUnit`, so that invocations were `new \\DrupalJUnit\\Parser()`) or that the namespace was generic but the classname was specific (e.g. `new \\JUnitParser\\Parse\\Drupal()`). Probably the first one is more expected.\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L14\r\n\r\nUnused import?\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L20\r\n\r\nClass comment block looks like it's actually supposed to be for the `parse()` method. So a proper class comment needs to be added and the method comment needs to be augmented a bit to be more descriptive and cover all the parameters.\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L56\r\n\r\nIt seems like you actually need to encapsulate the `Serializer` instantiation within this class. You rely on the fact that it has the XML encoder within this class, but unless you add that yourself here, there's no guarantee that the XML encoder is even available to work with.\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L65\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L72\r\n\r\n`$type` can only be one thing, so it makes sense to either turn these second conditionals into `elseif` or switch the control structures to `switch` statements to short circuit if the first condition ends up being true.\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L70\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L73\r\n\r\nThese private methods only ever use the data from `$this->report`, so it's a little odd that they take it as an argument. I'd suggest that either the `build` methods just directly make use of the `report` class member or that the `parse` methods return the data that is then passed into the `build` methods via a method argument.\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L75\r\n\r\nRather than tacking on the extra ending HTML tags, it would be cleaner to include the full HTML as the constant with some sort of token for where you want to inject the real content. Then you can just do a `str_replace()` to inject the content. (This would even allow you to pull the HTML out of the class entirely and just have it in a standalone HTML file, if you wanted to get carried away.)\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L83\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L109\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L124\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L137\r\n\r\nAgain, method comment blocks are a little lacking. 😁 \r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L142\r\n\r\nYou can probably save yourself a little trouble by storing the scenario states in a specific bucket, so that it's easier to pull out only what you want. (Rather than checking if the item you're looking at is an array.) i.e.\r\n```$this->report[$testsuite['@name']]['stats'] = [ ...blah...```\r\nThen this loop could be simplified down to\r\n```\r\nforeach ($node['stats'] as $key => $value) {\r\n  $this->html .= '<tr><td>' . $key . '</td><td>' . $value . '</td></tr>';\r\n}\r\n```\r\n\r\nhttps://github.com/ndrake0027/drupaljunitparser/blob/d1cdfd44fb49a14a33ff8c7f8553a4c5ad5b2bed/src/Parser.php#L150\r\n\r\nThis is kinda weird. Since we always need one, how about we just do it outside the loop? That way we don't have this `$header` flag thingy. e.g.\r\n```\r\n$this->html .= '</table><br><table>';\r\nif( count($node)) {\r\n  $this->html .= '<tr><th>' . implode('</th><th>', array_keys($node[0])) . '</th></tr>';\r\n}\r\nforeach ($node as $key => $test) {\r\n```"},"comment":{"url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/comments/385803530","html_url":"https://github.com/ndrake0027/drupaljunitparser/issues/1#issuecomment-385803530","issue_url":"https://api.github.com/repos/ndrake0027/drupaljunitparser/issues/1","id":385803530,"user":{"login":"ndrake0027","id":38641978,"avatar_url":"https://avatars2.githubusercontent.com/u/38641978?v=4","gravatar_id":"","url":"https://api.github.com/users/ndrake0027","html_url":"https://github.com/ndrake0027","followers_url":"https://api.github.com/users/ndrake0027/followers","following_url":"https://api.github.com/users/ndrake0027/following{/other_user}","gists_url":"https://api.github.com/users/ndrake0027/gists{/gist_id}","starred_url":"https://api.github.com/users/ndrake0027/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ndrake0027/subscriptions","organizations_url":"https://api.github.com/users/ndrake0027/orgs","repos_url":"https://api.github.com/users/ndrake0027/repos","events_url":"https://api.github.com/users/ndrake0027/events{/privacy}","received_events_url":"https://api.github.com/users/ndrake0027/received_events","type":"User","site_admin":false},"created_at":"2018-05-01T21:59:30Z","updated_at":"2018-05-01T21:59:30Z","author_association":"OWNER","body":"Everything should be good now."}}
{"actor":{"display_login":"ndrake0027"}}