Test instructions:
1. Install com_patchtester.zip (https://github.com/joomla-extensions/patchtester/releases)
2. Go to the Settings of the Patch tester and put in a random username and a password that contains a @ in it.
3. Save the Settings and go back to the Patch tester, you will see the error Could not resolve host at and part of the password.
4. Apply the patch
5. Try to load the listing again and now you will get the message Bad Credentials unless your real Github password does already contain a @. This means the URL was fine and Github received the username and password.
You may blame the J!Tracker Application for transmitting this comment.

Travis fails. Needs chnages in test
See
https://travis-ci.org/joomla/joomla-cms/jobs/21794693

Hi Jean-Marie,

Could you help us a bit on how to get this fixed with Travis CI? I can see that there are 2 different unit tests of Travis that fail because of this change. Would the pull request of Roland need to include a patch of those tests as well? I guess so.

Jisse (Yireo)

Surely the changes to the Travis tests have to be in a separate pull
request so that they can be merged first and then this PR would be tested
against the new tests. If the test are in the same PR then the code here
would still be tested against the old tests.
On 29 Mar 2014 09:39, "Yireo" notifications@github.com wrote:

Hi Jean-Marie,

Could you help us a bit on how to get this fixed with Travis CI? I can see
that there are 2 different unit tests of Travis that fail because of this
change. Would the pull request of Roland need to include a patch of those
tests as well? I guess so.

Jisse (Yireo)

Reply to this email directly or view it on GitHub#3387 (comment)
.

That would only introduce more of those situations where your PR fails because unrelated tests fail.
What I have seen sofar is that when your commit fails Travis, you either change your change or change the test.

As far as I know, new tests merged at the same time as the rest of a PR does make Travis OK

Exactly, Tests have to be updated together with the changed class in the same PR.

Understood. Thanks all for the clear thinking. A PR needs to be checked with the related unit test, and when that unit test fails the PR needs to include the change in the unit test as well (or the PR needs to be modified to make sure the existing unit test still changes).

Superseded by PR: #1535. Once that is merged we can check if this change is still needed.

Hello,
some observations:

First I agree completely that setting the credentials in the header is much better - no doubt about it.

Second, I believe you can achieve this right now using some custom code on your side like:

$username = 'fooUser';
$password = 'fooPass';

$headers = array(
  'Authorization' => 'Basic ' . base64_encode($username . ':' . $password)
);

$options = new JRegistry;

$options->set('headers', $headers);

$gitHub = new JGithub($options);

$test = $gitHub->account->getRateLimit();

var_dump($test);

Obviously this functionality would better go into the (J)GitHub classes... I believe that a better place to do this would be in JGithubHttp class. You could do this right in the constructor without adding a parameter to every method call ;)

Last bu not least... there is #1535 which contains a (almost) completed API. Maybe you could give it a bump so we can combine efforts