refactor tests, update dependencies and Project API by mathieudutour · Pull Request #401 · github-tools/github · GitHub

fix lint issue
update dependencies (:boom: breaking change)
Fix getContributors method URL (:boom: breaking change) (fix(repository): Fix getContributors method URL. Add getContributorStats method. #393)
Add getContributorStats method (fix(repository): Fix getContributors method URL. Add getContributorStats method. #393)
Issue.{get/delete/edit/list} for labels (feat: Issue get/delete/edit/list for labels #391)
Project API (both org and repo)
remove Promise polyfill (:boom: breaking change, drop support for node 0.12 without polyfill)
rename michael/github to github-tool/github
add support for preview APIs
add updateStatus and updateRepository method (feature(repository): add updateStatus and updateRepository method #390)
remove updatePullRequst (:boom: breaking change)
add user.getEmails
refactor tests to not use TestRepo (and make them pass)
add test coverage (refactor tests, update dependencies and Project API #401 (comment))

That's a lot of changes. Tell me if you are interested, I'll clean the history and the package.json

Current coverage is 90.02% (diff: 93.84%)

No coverage report found for master at 78a98b1.

Powered by Codecov. Last update 78a98b1...f998a39

Thanks for the great work!

The only question I have is what did you brea? I don't see what changes require a major version bump.

How much longer is node4 supported? Maybe we don't drop support for node4 just yet.

we are not dropping the support for node4, just node 0.12 and earlier. I removed it from the test cause there were some issues with running the test 3 times in parallel. But maybe we should test on node4 only

This may be solved by running them serially: https://docs.travis-ci.com/user/customizing-the-build#Limiting-Concurrent-Builds

Was moved to org

Lolz

@clayreimann You were asking about a breaking change that would necessitate a major version bump. This would be it.

acceptHeader?

the header is call Accept so I wasn't sure. I have no strong feeling about it

I think @clayreimann is getting at the usage of PascalCasing instead of camelCasing.

yes I understood

Is this diff still relevant?

it's not, I will roll back those changes. I wasn't sure you were interested in merging such a PR so I waited for that first ;)

👍 Yay conciseness!

Isn't this done by CI?

indeed, I'll roll back also

What's the reasoning for not using the latest version of npm?

What's the thought behind removing all this documentation? Are you proposing we drop the link to the generated Docs?

It's not removed, it's rearranged to fit https://github.com/noffle/art-of-readme

Why downgrade from ECMA2015?

hum, because for people who are not using babel/ES6, it's an additional cognitive load which can be avoided. For people using ES6, I'm quite sure they know how to use const instead of var.
But again, no strong feeling and happy to roll back if you prefer

My opinion is that ECMA2015 is spec now, so there's no need to downgrade.

sure, but var is not deprecated in anyway, so if we can help people understand the repo, I think it's best, regardless of wether it's matching style guide or not, don't you think? (see #392)

My idea was, and hope is, that we can have at least one example of each of: require vs import and then vs cb to give people the full flavor of the API.

That being said I think that perhaps that means most of our examples are import + then with an example included toward the end of require + cb.

That might be a solution. Feel free to commit on directly here

Again downgrading from ECMA2015?

Another downgrade from ECMA2015, I'll stop mentioning all the ones I see now.

@clayreimann You were asking about a breaking change that would necessitate a major version bump. This would be it.

Mocha allows you to return a promise chain instead of passing the done callback.

I generally find arbitrary delays like this in tests to be brittle. Perhaps a better solution is to poll for the repo until it comes back successfully.

My opinion is that ECMA2015 is spec now, so there's no need to downgrade.

I think @clayreimann is getting at the usage of PascalCasing instead of camelCasing.

Why the wait?

Sometimes the writeFile succeed even though github didn't really create the file yet and the getContents call fails with 404. Wait is there to give time to Github to settle down

Blind wait's a generally very brittle. Can you pull github for the file and if it doesn't show up within 20 seconds then fail?

Indeed, I'll see if I can write an easy to use helper. (This can be done in another PR though)

Agreed, I'm fine with it in another PR as well.

For the changelog generation should we have all these commits squashed with the commit message chore: fix tests

There's nothing here that I really object to, @mtscout6 look good to you too?

Just the rebase and squash for the change log generation. If that's still a thing for this repo.

How is the changelog generation working exactly?

Changelog generation is manual

…

On Nov 29, 2016, 5:45 PM -0600, Mathieu Dutour ***@***.***>, wrote: How is the changelog generation working exactly? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (#401 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA4w94TztRRlcNlX7iiIVQ6TgwtIZtIpks5rDLkYgaJpZM4K27nr).

Very well, I thought it was auto generated from the commit messages. Never mind then this is good to go.

mathieudutour added 29 commits November 18, 2016 12:35

add getContributorStats

a8ae05f

add labels methods

f5e6fa3

add updateStatus and updateRepository method

26fa048

nice readme

7c1684e

add project api and fix tests

a5e3dbb

3.0.0

cff3973

improve release script

4f46d10

3.0.1

83292fd

remove prepublish script

332456b

request all pages for cards

8b8c4fd

3.0.2

0505bb3

add getEmails

68ef1f6

3.1.0

cda2c42

remove polyfill

99c1aeb

3.1.1

0c5546f

drop support for node < 4

be44fd1

do not hardcode project preview in requestable

c12ad57

3.2.0

308aced

test on node6 only

3ec2837

add coverage

21e50ec

fix lint

2a0bdf2

fix travis.yml

ee8af3f

fix tests

5418512

fix scope

71a6326

add tests for project

5dd868f

add some timeout to settle

077736b

fix _request204or404

55db033

remove clearRepo

068476d

fix typo

97ac33f

mathieudutour mentioned this pull request Nov 18, 2016

WIP: Fix linting that is failing the CI #400

Closed

remove unnesserary setTimeout

1605b89

clayreimann reviewed Nov 20, 2016

View reviewed changes

mathieudutour added 7 commits November 21, 2016 11:53

fix conflicts

a80922c

fix callbackErrorOrThrow

121066c

add some delay

9877d9e

add some delay

45a6ef3

add more delay

fe146a5

test on node 4 and 5

c9de004

add more waiting

e1ae825

mtscout6 reviewed Nov 21, 2016

View reviewed changes

mathieudutour added 3 commits November 21, 2016 14:15

fix concurrency issues

fd5d57b

delete created org project

39dc920

fix typo

ac41f60

mtscout6 reviewed Nov 21, 2016

View reviewed changes

mathieudutour added 2 commits November 21, 2016 22:47

wait more

e0bbf7e

fix typo

f998a39

mtscout6 reviewed Nov 22, 2016

View reviewed changes

clayreimann merged commit 6ec1d0b into github-tools:master Nov 30, 2016

               node_js:
                 - '6'
-                - '5'
-                - '4'

-                  * @param {number|string} number - the number of the pull request to update
-                  * @param {Object} options - the pull request description
-                  * @param {Requestable.callback} [cb] - will receive the pull request information
-                  * @return {Promise} - the promise for the http request

-                "name": "github-api",
-                "version": "2.4.0",
+                "name": "@mathieudutour/github-api",
+                "version": "3.2.0",

-                             (found, member) => member.login === testUser.USERNAME || found,
-                             false
-                          );
+                          const hasTestUser = members.some((member) => member.login === testUser.USERNAME);

+              # switch back to master, build and publish
+              git checkout master
+              npm run build
+              npm publish

+              ## GitHub Tools
+              The team behind Github.js has created a whole organization, called [GitHub Tools](https://github.com/github-tools),
+              dedicated to GitHub and its API. In the near future this repository could be moved under the GitHub Tools organization

                     const url = this.__getURL(path);
-                    const headers = this.__getRequestHeaders(raw);
+                    const AcceptHeader = (data || {}).AcceptHeader;

               cache:
                 directories:
                   - node_modules
-              before_install: npm install -g npm@latest

-              dedicated to GitHub and its API. In the near future this repository could be moved under the GitHub Tools organization
-              as well. In the meantime, we recommend you to take a look at other projects of the organization.
-              ## Samples

               ```javascript
-              import GitHub from 'github-api';
+              var GitHub = require('github-api');

               // basic auth
-              const gh = new GitHub({
+              var gh = new GitHub({

               });
-              const me = gh.getUser();
+              var me = gh.getUser(); // no user specified defaults to the user for whom credentials were provided

                  let remoteIssues;
-                 before(function() {
+                 before(function(done) {

                       .getUser()
                       .createRepo({name: testRepoName})
-                      .then(wait())
+                      .then(wait(5000))

                     it('should write to repo', function(done) {
                        remoteRepo.writeFile('master', fileName, initialText, initialMessage, assertSuccessful(done, function() {
-                          remoteRepo.getContents('master', fileName, 'raw', assertSuccessful(done, function(err, fileText) {
+                          wait().then(() => remoteRepo.getContents('master', fileName, 'raw',

refactor tests, update dependencies and Project API by mathieudutour · Pull Request #401 · github-tools/github · GitHub

Uh oh!

refactor tests, update dependencies and Project API - #401

refactor tests, update dependencies and Project API#401clayreimann merged 42 commits intogithub-tools:mastergithub-tools/github:masterfrom mathieudutour:mastermathieudutour/github:masterCopy head branch name to clipboard

Conversation

mathieudutour commented Nov 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Nov 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 90.02% (diff: 93.84%)

Uh oh!

clayreimann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathieudutour Nov 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

refactor tests, update dependencies and Project API#401
clayreimann merged 42 commits into
github-tools:masterfrom
mathieudutour:master

mathieudutour commented Nov 18, 2016 •
edited
Loading

codecov-io commented Nov 18, 2016 •
edited
Loading

mathieudutour Nov 21, 2016 •
edited
Loading

mtscout6 commented Nov 29, 2016 •
edited
Loading