Skip to content

Conversation

@jgierer12
Copy link

No description provided.

@jgierer12
Copy link
Author

@watson friendly ping 🙂

@sibiraj-s
Copy link
Collaborator

is there any documentation available for this. @jgierer12 .?

exports.isPR = null

function includes (str, search) {
return !!~str.indexOf(search)

Choose a reason for hiding this comment

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

Make this less clever and more explicit.

-   return !!~str.indexOf(search)
+   return str.indexOf(search) > -1;

{
"name": "Heroku",
"constant": "HEROKU",
"env": { "env": "NODE", "includes": "heroku" }

Choose a reason for hiding this comment

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

In the test, you check for this value: /app/.heroku/node/bin/node. Is that a constant? Could you check for that value explicitly instead of any value with heroku in it?

@shian15810
Copy link

shian15810 commented Jun 23, 2022

@sibiraj-s After more than a year without any update, for Heroku support, is it okay for me to create another PR to supersede this?

@sibiraj-s
Copy link
Collaborator

Thanks @shian15810 PR's always welcome. And it is good to have the approach taken is somewhere documented by Heroku. So it won't break randomly.

@sibiraj-s
Copy link
Collaborator

Sorry for the long hold. But Closing this in favour of #95.

@sibiraj-s sibiraj-s closed this Nov 11, 2022
sibiraj-s pushed a commit that referenced this pull request Nov 13, 2022
Based off of the work started in #34
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.

4 participants