Skip to content

Conversation

@RubenVerborgh
Copy link
Contributor

@RubenVerborghRubenVerborgh 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
ContributorAuthor

(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
ContributorAuthor

@dmitrizagidulin Documentation added, ready for review.

Copy link
Contributor

@dmitrizagidulindmitrizagidulin 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.

@dmitrizagidulindmitrizagidulin merged commit 66e8f8a into dz_oidcAug 11, 2017
@dmitrizagidulindmitrizagidulin deleted the feature/startup-script-test branch August 11, 2017 18:08
Sign up for freeto 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

@RubenVerborgh@dmitrizagidulin