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

Added project-repository-files docs page #23

Merged
merged 4 commits into from
Jan 19, 2018
Merged

Added project-repository-files docs page #23

merged 4 commits into from
Jan 19, 2018

Conversation

arsdehnel
Copy link
Contributor

Hi @jdalrymple I like this project over some other gitlab api projects and like that you're trying to document things within the project rather than assuming people can figure out the details of how to make the calls by reading the code.

As I documented this call that I'm making I can't help but wonder if the encodeURIComponent should be part of this package rather than having users have to wrap their path in that. I'd be happy to make that change but wanted to run it by you first. Thoughts?

@jdalrymple
Copy link
Owner

Thank you! It was one of the main problems I ran into when I was trying to use the GL API. I figured adding a few extra docs couldn't hurt lol. I think it should be part of the package for sure! It would definitely help those using the library by removing the annoying step of making their paths encoded.

@arsdehnel
Copy link
Contributor Author

Cool. I can add that change to this PR and then also update this docs section to reflect that change. might also see about adding more to this page so it's not just the lonely one call on there.

@jdalrymple
Copy link
Owner

Sounds good! Thanks again!!

@arsdehnel
Copy link
Contributor Author

hey @jdalrymple wondering if you have recommendations on a setup for development? I'm still quite the noob on working on npm packages locally.

@arsdehnel
Copy link
Contributor Author

got a working setup and documented it in the readme. not sure the current best practice but looks like either npm link or linklocal are the leading contenders. If you have suggestions on the best setup I'll do that and then document what I've got.

I also updated the code and my docs contribution, feedback welcome on those changes.

@jdalrymple
Copy link
Owner

Ill check through your work tonight. At a glace it looks great!! As for the linking, it would probably be best to stick with the npm script as a standard, so npm link.

@arsdehnel
Copy link
Contributor Author

@jdalrymple these are indeed pretty similar lines but this pattern is used throughout the project so I'd hate to do something new here. Thoughts on changing how I did this vs following the pattern of the project?

@jdalrymple
Copy link
Owner

Still haven't had a chance to read through your PR yet due to some personal commitments :(, will do soon though!!

@jdalrymple
Copy link
Owner

Looks good! Ill try and clean up the code climate stuff!

@jdalrymple jdalrymple merged commit 1f78327 into jdalrymple:master Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants