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

Reformulate installation guidelines and add section about proxies #783

Merged
merged 3 commits into from
May 16, 2017

Conversation

honzajavorek
Copy link
Contributor

🚀 Why this change?

Installation of Dredd in restricted environment isn't completely straightforward. I added a section about the necessary steps. At the same time, I tried to simplify other sections as well and to make them more use-case-oriented.

📝 Related issues and Pull Requests

✅ What didn't I forget?

  • To write docs
  • To write tests - N/A
  • To put Conventional Changelog prefixes in front of all my commits and run npm run lint

Aim of the changes is to be less verbose about technical details
and to be more action-oriented then prose-oriented in the text.
@honzajavorek
Copy link
Contributor Author

I'm requesting review from both @abtris (to verify the steps to set proxy are correct) AND @miiila (to review the rest of the text and correct English).

Copy link
Contributor

@miiila miiila left a comment

Choose a reason for hiding this comment

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

I've added some comments about the language, but I am not the native, so I could introduce more mistakes than I fixed ;-)


Dredd sometimes issues a pre-release version to test experimental features or to ensure that significant internal revamp of existing features didn't cause any regressions. It's possible to use `npm install dredd@stable` to avoid installing the pre-release versions. However, for most of the time, there are no pre-releases and the `stable` tag just points to the latest version.
### Why I'm Seeing Network Errors?
Copy link
Contributor

Choose a reason for hiding this comment

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

Why am I seeing


#### Compiled vs pure JavaScript
If you're in restricted network (VPN, firewall, proxy), it's possible you see errors similar to the following ones:
Copy link
Contributor

Choose a reason for hiding this comment

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

in a restricted network... you can see errors

@@ -86,6 +86,17 @@ Also mind that CoffeeScript is production dependency (not dev dependency),
because it's needed not only for compiling Dredd package before uploading
to npm, but also for running user-provided hooks written in CoffeeScript.

### Compiled vs pure JavaScript

Dredd uses [Drafter][] for parsing [API Blueprint][] documents. Drafter is written in C++11 and needs to be compiled during installation. Since that can be problematic for some environments and leads to a lot of troubleshooting, there's also pure JavaScript version of the parser, [drafter.js][]. While drafter.js is fully equivalent, it can have slower performance. That's why there's [drafter-npm][] package, which first tries to compile the C++11 version of the parser and if it's unsuccessful, uses the JavaScript equivalent.
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this can cause a lot of troubles in some environments, there's also...
Drafter.js is fully equivalent, but it can have slower performance.
Therefore there's...
which tries to compile the C++11 version of the parser and uses the JavaScript equivalent in case of failure


Dredd uses [Drafter][] for parsing [API Blueprint][] documents. Drafter is written in C++11 and needs to be compiled during installation. Since that can be problematic for some environments and leads to a lot of troubleshooting, there's also pure JavaScript version of the parser, [drafter.js][]. While drafter.js is fully equivalent, it can have slower performance. That's why there's [drafter-npm][] package, which first tries to compile the C++11 version of the parser and if it's unsuccessful, uses the JavaScript equivalent.

Dredd depends on the [drafter-npm][] package. That's why you can see `node-gyp` errors and failures during Dredd installation, although after the installation is done, Dredd seems to normally work and correctly parses API Blueprint documents. Usual problems leading to the JavaScript version of the parser being used as fallback:
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the reason why you can see node-gyp errors and failures...
even though installation ended correctly and Dredd...

Last sentence doesn't make sense for me - are you trying to fix this issues? Is it necessary if it seems to be working? I would appreciate more explanation on the goal you want to achieve with the following list.

@honzajavorek honzajavorek force-pushed the honzajavorek/restricted-network branch from 3c53fd9 to 4c9db42 Compare May 16, 2017 13:17
@honzajavorek
Copy link
Contributor Author

@abtris @miiila Thanks!

@miiila would you please re-read the "Compiled vs pure JavaScript" part, please? I tried to make it more clear.

@honzajavorek honzajavorek force-pushed the honzajavorek/restricted-network branch from 4c9db42 to fb14e6a Compare May 16, 2017 13:22
Copy link
Contributor

@miiila miiila left a comment

Choose a reason for hiding this comment

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

📖

@miiila miiila merged commit fdd1ce2 into master May 16, 2017
@honzajavorek honzajavorek deleted the honzajavorek/restricted-network branch May 16, 2017 14:27
@honzajavorek honzajavorek restored the honzajavorek/restricted-network branch May 16, 2017 14:29
@honzajavorek honzajavorek deleted the honzajavorek/restricted-network branch May 16, 2017 14:55
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.

3 participants