Skip to content

Remove redundant pathing #19

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Remove redundant pathing #19

wants to merge 1 commit into from

Conversation

jasonkarns
Copy link
Contributor

Manually prepending $(npm bin) to dependencies is redundant in npm scripts. Npm already prepends $(npm bin) to $PATH before executing scripts.

@jasonkarns
Remove redundant pathing
8f58f35 
Manually prepending `$(npm bin)` to dependencies is redundant in npm scripts. Npm already prepends `$(npm bin)` to `$PATH` before executing scripts.
@mikeproeng37
Copy link
Contributor

Thanks for the fix @jasonkarns !

Please take the time to go through our https://github.com/optimizely/javascript-sdk/blob/master/CONTRIBUTING.md doc and go through the steps outlined there.

@mikeproeng37
Copy link
Contributor

@jasonkarns have you had a chance to sign our CLA? Also, please open a PR against our devel branch.

@jasonkarns
Copy link
Contributor Author

Also, please open a PR against our devel branch.

Please don't take personal offense to this. But no.

Optimizely is a for-profit company. I provided useful improvements via the community established norms and conventions. If optimizely wishes to erect obstacles so as to make it difficult for people to provide unpaid improvements to their for-profit product, then that is their choosing. But I will not spend more of my unpaid time because Optimizely chooses to ignore those norms and conventions.

@jasonkarns
Copy link
Contributor Author

To clarify: you say you want devel to be the default branch off which PRs are based. (Which is in and of itself, bucking the entire git community's convention, so wrong off the start, IMO.) But you still have master configured as the default branch within github. Follow the conventions, or at the very least, configure github to use your preferred default (against best practice). Lower the barrier to contribution, or why bother open sourcing this at all?

@mikeproeng37
Copy link
Contributor

No offense taken @jasonkarns. You have raised excellent points and your feedback is appreciated. We will work to minimize the friction for developers to contribute to our codebase. Stay tuned.

@mikeproeng37
Copy link
Contributor

@jasonkarns we've adjusted our open source process so that master is now our default branch. If you'd still like this PR to be merged, please resolve the conflicts.

@mikeproeng37
Copy link
Contributor

Closing due to unresponsiveness.

@jasonkarns jasonkarns mentioned this pull request May 25, 2017
Merged
mjc1283 added a commit that referenced this pull request Apr 22, 2019
@mjc1283
fix (datafile management): Fix unhandled onReady promise rejections i…
8db426d 
…n unit tests (#19)

Summary:

With changes for datafile management, we introduced Promise rejections that need to be handled. Add handlers to these expected errors.

Test plan:

Run unit tests with changes from this branch and master branch (which exits the test process on unhandled rejections)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jasonkarns @mikeproeng37