[#1535] - Add refactored JGitHub package
- Closed
- 1 Apr 2014
- Medium
- Build: master
- # 1535
- Diff
- elkuku:jgithub
- Success default The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This is a package that didn't made it into the platform.
The "current" version can be found here: https://github.com/joomla/joomla-framework/tree/staging/src/Joomla/Github
This may still need some cleanup. I'm putting it here "on the table" so interested people could get their hands and/or eyes on it.
References:
From joomla/joomla-framework#128
This is a follow up from joomla/joomla-platform#1820
Structure amd Documentation
The API is organized using the same structure as the API documentation on GitHub
Example
<tr>
<td>Repositories</td>
<td></td>
<td></td>
<td><a href="http://developer.github.com/v3/repos/">developer.github.com/v3/<b>repos</b></a></td>
</tr>
<tr>
<td><code>repositories-></code></td>
<td></td>
<td><code>get()</code></td>
<td><a href="http://developer.github.com/v3/repos/#get">developer.github.com/v3/<b>repos/#get</b></a></td>
</tr>
<tr>
<td>Repositories</td>
<td>Comments</td>
<td></td>
<td><a href="http://developer.github.com/v3/repos/comments/">developer.github.com/v3/<b>repos/comments</b></a></td>
</tr>
<tr>
<td><code>repositories-></code></td>
<td><code>comments-></code></td>
<td><code>get()</code></td>
<td><a href="http://developer.github.com/v3/repos/comments/#get-a-single-commit-comment">developer.github.com/v3/<b>repos/comments/#get-a-single-commit-comment</b></a>
</td>
</tr>
</tr>
| Package | Sub Package | Method | Documentation |
|---|
Hint: Take a look at the Joomla! API Documentation and compare to the GitHub API Documentation (the right navigation menu).
Backwards compatibility
All methods have been moved:
Proxy methods have been added to assure full backwards compatibility.
Additional resources
| Platform | Framework |
|---|---|
| API Doc | API Doc |
| UT coverage | UT coverage |
IIRC, proper use through the API isn't affected through this refactoring. Directly instantiating the child classes of JGithubObject would be, and this action is discouraged. There's some class names that changed and methods realigned to get our GitHub package in line with the GitHub v3 API, but that's really it.
My personal opinion, we merge it for 3.2 with advanced notification of this change in the off chance someone is actually using this package.
Actually the method names and some "access" to them have changed - all of them.. I updated the description with the original PR. This also includes an incompatibility chart.
IMHO creating proxy methods to the old functions would cause more confusion then help, but it should be possible.
TBH I think @wilsonge is right. I don't think the Joomla! developer community can afford the loss of yet another developer because of yet another J!-B/C-Issue™
So I rewrote my proposal, left the old unconflicting methods in place, created proxy methods for the overlappings and added proper deprecation notes.
This should now be fully backwards compatible - Happy GitHubbing 
Awesome :D. I think we should have some JLog statements marking the deprecated functions. Kind of like we have here: https://github.com/joomla/joomla-cms/blob/master/libraries/joomla/database/database.php#L33
Sorry for being a PITA!!
Your comments are everything else but a PITA, but I'd beg to differ here. IMHO the calls to JLog do not provide any added value, instead they are slowing down the code.
The new API is now fully backwards compatible, providing proper deprecation messages with alternatives in every modern IDE.
If you think the Joomla! CMS or whatever is processing those JLog messages could provide any additional help, please feel free to add the required code.
@elkuku Would you mind updating this? Not sure what's up but it's showing as no mergable.
Overall we have been logging deprecations but I agree that it's a tradeoff on performance. I'm not sure there is a great answer although of course the performance hit is overcome by updating the code.
Hello,
it's been almost three month since the last "activity" on this issue.
Is there anything else that should be done here ?
Can this __DEPLOY_VERSION__ thingy be used here for the @since tags ?
It would be "nice" if the Joomla! project would have some "rule" saying that a PR should be either merged or closed after a "certain" amount of time of inactivity --- three month being definitely too much.
Merry Christmas....
Sorry about that, fell off my radar with all the other stuff going on the
last couple of months. I think we're ready to announce our next plans,
look for an update very soon that will affect this PR :-)
On Thu, Dec 19, 2013 at 8:34 AM, Nikolai Plath notifications@github.comwrote:
Hello,
it's been almost three month since the last "activity" on this issue.Is there anything else that should be done here ?
Can this DEPLOY_VERSION thingy be used here for the @since tags ?It would be "nice" if the Joomla! project would have some "rule" saying
that a PR should be either merged or closed after a "certain" amount of
time of inactivity --- three month being definitely too much.Merry Christmas....
—
Reply to this email directly or view it on GitHub#1535 (comment)
.
Well, Michael, for all the bad stuff that's happening, you are the last person I'd been thinking of for blaming, because of all the good stuff you're bringing to the Joomla! project.
So.. I'm going to look forward for this update whatever it might be
look for an update very soon that will affect this PR :-)
Well I know that soon was meant as JSoon™ - but just to be sure: "Did I miss this update ? "
Haven't seen anything. So could/should we merge this into 3.3 dev branch and get this in? At least with 3.3 the small number of deprecations becomes less of an issue
Ya, we'll get this into 3.3. Might need to nag me about it again in a couple of days the way my memory is at times LOL
Bump Michael
On my todo list for this weekend. Thanks for the bump LOL
:)
Just a bump :)
I tried pulling this down manually to merge it, failed.
error: patch failed: libraries/joomla/github/gists.php:1
error: libraries/joomla/github/gists.php: patch does not apply
error: patch failed: libraries/joomla/github/issues.php:1
error: libraries/joomla/github/issues.php: patch does not apply
error: patch failed: libraries/joomla/github/pulls.php:1
error: libraries/joomla/github/pulls.php: patch does not apply
error: patch failed: libraries/joomla/github/users.php:1
error: libraries/joomla/github/users.php: patch does not apply
I'll take a look later...
Sent from my Android =;)
On Mar 31, 2014 8:22 PM, "Michael Babker" notifications@github.com wrote:
I tried pulling this down manually to merge it, failed.
error: patch failed: libraries/joomla/github/gists.php:1
error: libraries/joomla/github/gists.php: patch does not apply
error: patch failed: libraries/joomla/github/issues.php:1
error: libraries/joomla/github/issues.php: patch does not apply
error: patch failed: libraries/joomla/github/pulls.php:1
error: libraries/joomla/github/pulls.php: patch does not apply
error: patch failed: libraries/joomla/github/users.php:1
error: libraries/joomla/github/users.php: patch does not applyReply to this email directly or view it on GitHub#1535 (comment)
.
Copyright dates.......
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-04-01 20:45:18 |
Ahh crap...
I was about to ask if I should change the target branch ;)
Whilst this should be used at some point - quickly glancing over the code this isn't backwards compatible with the old class yet. So either we'd need to redirect to existing classes OR wait for Joomla 4.x to enact this refactoring