search

parro-it / electron-google-oauth

Google api access token in electron
MIT License
56 stars 16 forks source link

Add argument to pass browser window params #1

Closed luigiplr closed 9 years ago

luigiplr commented 9 years ago

And bump deps

parro-it commented 9 years ago

Hi, thank you for the PR. I have some issues with it, if you can solve them I can merge it.

1) Why you unhide the dist folder? Since it is generated by Babel, I prefer to avoid put it in git. 2) Make the BrowserWindowParams arguments default to {'use-content-size': true } to keep compatibility with previous versions. 3) Maybe it's better to call it browserWindowParams since it's not a class?

luigiplr commented 9 years ago

hi @parro-it :#1 I'll rehide the dist folder, unhid it as i and some others had problems without it. #2 & #3 sure

parro-it commented 9 years ago

Fine. What kind of problem you had? It should be included in npm package

parro-it commented 9 years ago

If it's not, it's my fault, but it should

luigiplr commented 9 years ago

@parro-it when using the dependency as a direct git type aka "electron-google-oauth": "git+https://github.com/parro-it/electron-google-oauth.git" it does not run the prepublish task nor is dist/ generated. this makes npm throw a error and fail to install the dep.

Also no prob the dependency itself is awesome!

parro-it commented 9 years ago

Ah ok understood. There is a particular reason you're using the git scheme dep?

luigiplr commented 9 years ago

@parro-it it was mainly used for quick testing of edits without having to publish the dependency to the registry. However other uses would be if you wanted to keep the code to a dep private without paying for more on npmjs.com

parro-it commented 9 years ago

That's fine, I already used them too, but on my own private gh repos, I was asking if there is a particular reason you have to use them on this repo

luigiplr commented 9 years ago

No particular reason. Simply for fast testing :)

parro-it commented 9 years ago

Ok, in this case I prefer to keep it hidden. One last thing, then I'll merge: could you please rename the arg also in the exported folder, and also commit deletion of the dist folder you added? Thank you

luigiplr commented 9 years ago

@parro-it should be renamed, & removed.