Added an additional JSON diff output to the ApiXmlDocReader

[swill]

The original TXT diff format is very hard to work with if you are trying to programmatically use the output to generate documentation. Previously, @pdion891 spent a lot of time trying to manually reformat this output in order for it to be represented in the Release Notes.

This pull request adds JSON as an output format as well as the existing TXT to simplify the ability for parsers and generators to use the output.

This new JSON format was used to generate the 4.9.0 release notes (that code will be contributed separately).

Old manually formatted output:
http://docs.cloudstack.apache.org/projects/cloudstack-release-notes/en/4.7.0/api-changes.html

New auto-formatted output using this new JSON output:
http://docs.cloudstack.apache.org/projects/cloudstack-release-notes/en/4.9.0/api-changes.html

I have included the output of the diff.txt as well as the diff.json to this PR so you can see the resulting output of both file formats (diff between 4.8 and 4.9).

diff.txt.zip
diff.json.zip

[rohityadavcloud]

Great idea, LGTM (based on the api-changes urls). @swill can we also sort the list of new APIs by name.

[swill]

I don't think this implementation should change. This simply outputs the results to a JSON object (which is unordered by definition). The generation of the documentation is separate and that code will have its own pull request as a helper script against the cloudstack-docs-rn repo. I can add the ordering logic to the generation script if we would like it.

This PR is the foundation for being able to use scripting to generate the API change docs, so this is a dependency for the other PR (not yet opened).

[swill]

Can we get another code review of this PR so we can get this merged so I can submit PRs to include the doc generation scripts to the docs repos which depend on this functionality?

[pdion891]

lgtm, that's usefull

[swill]

@jburwell can we merge this? Not sure what process you have in place right now. Thx...

[jburwell]

@swill any committer may merge so long as there is at least 1 code review LGTM, 1 test LGTM, and no -1s. I see 2 code review LGTMS. Since this PR is for docs, is there a way to test it? If not, LGTM.

[swill]

Well, it is to help generate the docs. I have included the txt and the new json output produced by this addition to the original post (OP). I have also used this code to create the release notes linked in the OP, so I would consider the code tested.

I have your blessing to merge this then?

[swill]

I rebased against the current master. I will merge this as soon as the status comes back green...

[karuturi]

@jburwell we didnt conclude on that discussion. Its not the policy as of now.

[swill]

@jburwell & @karuturi please advise. This PR should be ready to be merged...

[karuturi]

@swill sorry to hijack. This is not a -1 for you to commit. Please go ahead. I just want to make it clear that we as a community never concluded on that discussion. But, given how things are going in the project, I guess I dont mind either way.

[swill]

@karuturi no worries, I know the comment was not related to this specific PR and is related to the larger topic of how we are currently handling merge policy.

I have been a bit MIA for the last few weeks, so I have not kept up on this conversation. I am currently unclear on what our merge policy is right now, which is why I am asking. I know that as a CloudStack committer, I have the right to commit, but the free-for-all commit policy is also what made master unstable until 4.6. I am not sure what checks and balances we have in place to guarantee the stability of master right now. I hope we are not counting only on Travis since there were many cases when I was RM where Travis showed green, but bubble would catch problems...

I don't mind holding off on merging this if we would like to discuss a bit more in this issue...

[jburwell]

@karuturi @swill there is no discussion to be had. According to our bylaws, committers have the right to commit to any branch. Period. Full stop. If you would like to change it, propose a change to the bylaws.

[swill]

@jburwell we have not been operating that way since 4.6 for a reason. I understand it is that way in the bylaws, but the bylaws are not currently written to protect the stability of master. How are we protecting the stability of master right now??? If we are not protecting the stability of master, then a free-for-all is a liability to the project, regardless of what the bylaws say.

[jburwell]

@swill we never had a policy that only release managers could merge. Remi was active that he typically merged PRs before anyone else had a chance. The [release principles|https://cwiki.apache.org/confluence/display/CLOUDSTACK/Release+principles+for+Apache+CloudStack+4.6+and+up] to which we agreed only said that a PR needed 1 code review LGTM, 1 test LGTM, and no -1s. It also lays out how it should be done. Nowhere in that document does it restrict who can perform a PR merge. To take away that privilege requires a change to the bylaws. Personally, I don't recall a vote where I gave up my rights as a committer because I would have been a strong -1 on that notion.

[swill]

@jburwell since 4.6 we have effectively had the RM merge everything. I am not saying that is required, or even specified anywhere, but the reason we have been doing that was because the RM was actively maintaining the stability of master. Regardless of who is merging what, my question stands. How are we validating the stability of master right now? All the other details are basically irrelevant, this is the single most important question...

[jburwell]

@swill the RM is still maintaining the stability of master. I check that all non-security merges meet the criteria we have laid out. If/when I find one that does not, I will roll it back, and work with the contributors to address the deficiencies. The value of the process to which we have agreed is not who performs the merge operation, but the 2 LGTM threshold.

By definition, committers co-equals in the community. What makes @karuturi or myself any more capable/qualified committers to assess a PR and merge it? We simply volunteered to do help marshall releases. We have PRs that have languished simply waiting for one of us to have time to run a script when any other committer could have done it. If there are committers that you (or anyone else) feels have not fulfilled the trust they have been imparted, there are mechanisms for addressing those issues via PMC. Rather than restricting the flow of work, let's address those types of issues individually as the Apache Way is designed.

[swill]

@jburwell I feel like we are not discussing the same thing. I understand where you are coming from regarding the operational merging of PRs and I don't disagree with you. 2 LGTM with a 1 test and it gets merged (by someone)...

What I am trying to understand is how we are maintaining the stability of master. Just because two people give it a LGTM and Travis is green does not mean that it does not break something. There were a lot of PRs in 4.9 where Travis was green and CI still caught problems. I am trying to understand how we are currently ensuring the master branch stays stable...

[jburwell]

@swill other than verifying that the proper review has been done and a reasonable set of Marvin test cases have been run on the appropriate platforms, what else are you expecting?

[swill]

@jburwell ok cool. I didn't realize that we had Marvin tests running against hardware to validate the PRs. Is it just Travis using the simulator or do we have real hardware CI being run? Is this being done with Trillian?

[jburwell]

@swill where hardware are being varies by PR. In some cases, we have people running them in their labs and reporting results. Other cases, it's blueorangatan going through ShapeBlue Jenkins + trillian. But yes, PRs are getting tested on hardware.

(Sadly, we are still recovering from a dead storage array, so blueorangatan has been out of commission for a little bit),

[swill]

@jburwell ok cool. I will try to start getting my CI run against master every week to help make sure that master is happy. Hopefully that will help as well. :)

Alright, I think we have side barred this PR enough. I am going to merge it...

[karuturi]

@jburwell Thanks for the info on bylaws. Its not the first time we are discussing on how we are operating since 4.6. I understood that this is never going to conclude in any other way and I gave up(http://sethgodin.typepad.com/seths_blog/2016/08/putting-falsifiability-to-work.html)
Since 4.6, we are operating in a certain way and I do believe we should atleast conclude that discussion that anyone can commit(Thats my opinion and I am not looking for an argument here).

regarding the CI and @blueorangatan, the last time I checked, there is no proof there for me to believe what it says is right. I cant access the system(atleast the logs). The success rate(of it starting) is also very low for the PRs I tried. I understand its your internal CI and I appreciate the efforts. But, its not reliable yet.
signing off

[jburwell]

@karuturi the point is that there is no discussion about who can commit. It is in black and white in Section 2.3 of our bylaws and the Apache Way. As I said in my previous comment, no one ever agreed that only RMs could commit to release branches. We agreed to the criteria for a PR to be merged. By nature of his deep involvement, Remi ended up merging all PRs because he was so active. However, that circumstance did not imply that committers had relinquished their rights under our bylaws.

blueorganatan was under development until we had a hardware failure in the lab. Before development was interrupted, blueorganatan was posted detailed results to PRs with public exposure of the logs/results from the environment as the next item to address. When we resume development in the next week or so, exposing logs/results will be first item implemented.