Skip to content

Conversation

@RubenVerborgh
Copy link
Contributor

@RubenVerborgh RubenVerborgh commented Aug 11, 2017

The security setting associated with Node 8 compatibility (#518) means the server will only work with trusted certificates. To avoid this for testing purposes, the NODE_TLS_REJECT_UNAUTHORIZED environment variable should be set.

In order to facilitate that, this pull request adds a bin/solid-test executable. For consistency, it also renames the default executable to bin/solid from bin/solid.js, but the latter is symlinked.

Open questions:

  • Is solid-test an appropriate name? I thought of alternatives such as solid-insecure, but that seemed to strong. In any case, the executable is not exported in package.json.
  • Where should documentation for this feature be added? (In any case, to the changelog [Update CHANGELOG for version 4.0.0 #534].)

@dmitrizagidulin
Copy link
Contributor

I think naming it solid-test would carry across the right connotation. (We can also automatically prepend the DEBUG="solid:*" env variable, too, while we're at it).

As for documenting, aside from the CHANGELOG, I think we should also add it to the README, in the Command Line Usage / Running a Server sections. What do you think?

@dmitrizagidulin
Copy link
Contributor

I'm also not sure whether it's worth leaving solid-test out of the npm scripts..

@RubenVerborgh
Copy link
Contributor Author

(We can also automatically prepend the DEBUG="solid:*" env variable, too, while we're at it).

Maybe not, as people might like to make their own, more specific settings?

I think we should also add it to the README, in the Command Line Usage / Running a Server sections.

Will do!

I'm also not sure whether it's worth leaving solid-test out of the npm scripts.

Well, there's scripts and there is bin. I don't think it has a place in bin, but maybe in scripts. Then the question is what script? I thought about start (as this is commonly used), but there's also already a solid entry. Maybe a solid-test entry then?

@RubenVerborgh
Copy link
Contributor Author

@dmitrizagidulin Documentation added, ready for review.

Copy link
Contributor

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

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

Excellent, thanks.

@dmitrizagidulin dmitrizagidulin merged commit 66e8f8a into dz_oidc Aug 11, 2017
@dmitrizagidulin dmitrizagidulin deleted the feature/startup-script-test branch August 11, 2017 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants