FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

git_create_tag doesn't have the correct descriptor by mattyclarkson · Pull Request #430 · nodegit/nodegit · GitHub

Repository navigation

git_create_tag doesn't have the correct descriptor - #430

Closed
mattyclarkson wants to merge 10 commits into
nodegit:masterfrom
mattyclarkson:master
Closed

mattyclarkson wants to merge 10 commits into
nodegit:masterfrom
mattyclarkson:master

Conversation

Copy link
Copy Markdown
Collaborator

The following code doesn't work:

// Commit the bower.json file
.then(function() {
  return repository.createCommitOnHead(['bower.json'], identity, identity, 'release ' + version);
})

// Tag the new commit
.then(function(oid) {
  return nodegit.Commit.lookup(repository, oid);
})
.then(function(commit) {
  return nodegit.Tag.create(null, repository, version, commit, identity, 'version ' + version, 0);
})

This is because the git_create_tag needs to have the following descriptor:

"git_tag_create": {
  "isAsync": true,
  "args": {
    "oid": {
    "isReturn": true,
    "isSelf": false,
    "shouldAlloc": true
  },
  "return": {
    "cppClassName": "Number",
    "jsClassName": "Number",
    "isErrorCode": true
  }
},

johnhaley81 added this to the 0.3.0 milestone Feb 26, 2015

maxkorp commented Feb 26, 2015

Copy link
Copy Markdown
Collaborator

A note, should be able to skip the args/return with the isAsync set, at least, i'd try it without first, then add those if necessary.

Copy link
Copy Markdown
Collaborator Author

Ow. it failed tests 😢 will look through the logs.

Comment thread test/tests/tag.js Outdated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't really like that this test bundles both the creation and deletion together into a single test but if the tag isn't cleaned up the tests aren't repeatable.

Copy link
Copy Markdown
Collaborator

Looks like it failed linting. Try running npm test to see the linter errors.

Copy link
Copy Markdown
Collaborator Author

huh, the linting checks on appveyor are moaning about Promise being a bad name. but that is used across lots of the test files. Is that something I should fix up across all the files? Not sure why that didn't show up on my box.

Copy link
Copy Markdown
Collaborator Author

Oh, I didn't run jshint lib test/tests examples lifecycleScripts

Copy link
Copy Markdown
Member

Ah my mistake, can you force push a fix that doesn't use camelCase, it's all lower case per:

http://jshint.com/docs/options/#futurehostile

mattyclarkson force-pushed the master branch 2 times, most recently from 90725d0 to d7c9e04 Compare February 27, 2015 19:26

Copy link
Copy Markdown
Collaborator Author

Yay! 😃

Copy link
Copy Markdown
Collaborator

👍

The `git_tag_create*` functions expect there to be a Oid pointer as the
first parameter.
I'm not sure of how this should be handled at the moment. It will need
a similar descriptor as `git_tag_create` but maybe extra data defining
`buffer`.
A good convenience function for deleting a tag
This allows a user to easily create an annotated tag in a repository:

```
repository.createTag(oid, '0.0.0', 'version 0.0.0')
  .then(function(tag) {
    // The new tag is returned
  });
```
This allows new tags to be created and tested
This is a simple function that can create a new lightweight tag in a
repository. The same can be acheived by creating a new reference in
`/refs/tags/` but this performs libgit2 validation of tag names.

```
return repository.createLightweightTag(oid, 'foobar')
  .then(function(tag) {
    // The new tag is returned
  });
```
This flag fixes up the fact that we are polyfilling `Promise`:

```
var Promise = require('nodegit-promise');
```

Copy link
Copy Markdown
Collaborator

Manually merging this

Copy link
Copy Markdown
Collaborator

Manually merged via 094f240

maxkorp commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator

Awesome work @mattyclarkson

Copy link
Copy Markdown
Collaborator Author

@johnhaley81, sorry I didn't merge this on friday the build failed on appveyor and I had to leave for a weekend away! Thanks for merging it in manually.

Copy link
Copy Markdown
Collaborator

No worries :)

On Mon, Mar 2, 2015, 2:23 AM Matt Clarkson notifications@github.com wrote:

@johnhaley81 https://github.com/johnhaley81, sorry I didn't merge this
on friday the build failed on appveyor and I had to leave for a weekend
away! Thanks for merging it in manually.

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL