Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Orientjs upgrade and update to support orientjs v3.0.0 features . #14425

Closed
wants to merge 16 commits into from
Closed

Conversation

saeedtabrizi
Copy link
Contributor

@saeedtabrizi saeedtabrizi commented Feb 3, 2017

Please fill in this template.

  • Make your PR against the master branch.
  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run tsc without errors.
  • Run npm run lint package-name if a tslint.json is present.

Select one of these and delete the others:

If adding a new definition:

  • The package does not provide its own types, and you can not add them.
  • If this is for an NPM package, match the name. If not, do not conflict with the name of an NPM package.
  • Create it with npm run new-package package-name, not by basing it on an existing project.
  • tslint.json should be present, and tsconfig.json should have noImplicitAny, noImplicitThis, and strictNullChecks set to true.

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: <>
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "../tslint.json" }.

If removing a declaration:

  • If a package was never on DefinitelyTyped, you don't need to do anything. (If you wrote a package and provided types, you don't need to register it with us.)
  • Delete the package's directory.
  • Add it to notNeededPackages.json.

@dt-bot
Copy link
Member

dt-bot commented Feb 3, 2017

orientjs/index.d.ts

Checklist

@saeedtabrizi
Copy link
Contributor Author

saeedtabrizi commented Feb 3, 2017

All new feature of orientjs has declared in this definition update .

fix for query option params and correct if statement
- serverconfig  fixed
- fix serverconfig
interface Migration {
name: string;
server: Server;
Server: Server;
db: Db;
configure(config?: any): void;
up(): Promise<any>;
down(): Promise<any>;

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you remove this newline as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aozgaa , i correct to case of server ; Server is invalid and server is valid .

add some fix for migration and properties compatibilities
add some fix for support property and migration
@saeedtabrizi
Copy link
Contributor Author

Hi @aozgaa , I fixed some issues on orientjs.d.ts , index.d.ts . please merge this pull request .

@aozgaa
Copy link
Contributor

aozgaa commented Feb 7, 2017

Can you resolve the merge conflicts?

@saeedtabrizi
Copy link
Contributor Author

saeedtabrizi commented Feb 8, 2017

@aozgaa I have no any access to resolve it .

@zhengbli
Copy link
Contributor

@saeedtabrizi can you merge the conflict on your branch? Otherwise there is no way to merge it. Thanks!

@saeedtabrizi
Copy link
Contributor Author

@zhengbli I have no any conflict in my local branch and i have no any access to resolve because i have no write access to resolve this conflicts .

@zhengbli
Copy link
Contributor

so if pull the latest master branch, and do git merge master locally, would you see conflicts?

@jramsay
Copy link
Contributor

jramsay commented Mar 2, 2017

@saeedtabrizi: are you able to update this PR as per @zhengbli's instructions?

@saeedtabrizi
Copy link
Contributor Author

@zhengbli No , i have no detect any conflict in my code .

@mhegazy mhegazy added the Revision needed This PR needs code changes before it can be merged. label Mar 11, 2017
@paulvanbrenk
Copy link
Contributor

I tried to manually merge this, but it looks like your changes are based on a very old version of the definition file. Please consider manually re-applying the changes on a clean copy of master and submitting a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revision needed This PR needs code changes before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants