-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
Update index.js #106
Update index.js #106
Conversation
make basUrl parameter fnction
This is my first PR ever to any project. My motivation behind doing this is I have an Nginx server. Its a reverse proxy with caching system. Its acting like a CDN.
I hope I explained myself good. I'm not a professional. Sorry for mistakes. |
Thanks for the PR. It looks good to me. Few things:
|
@davimacedo when you say fix the link problems, you mean these:
I believe I can fix them. But I dont know how to write tests. Like I said I'm not a professional. I'm a law school student. And I do programming as a hobby. |
Sorry I said link but it is lint. For the test case, you can duplicate this section and add similar test cases but passing a function instead of a string. By doing this we will make sure that your addition will not only work but also nobody will break it in the future. |
fix-lint
fix lint
fix lint
fix lint
@davimacedo I fixed the lint problems. Now I will try to write some tests. I will edit test.spec.js file in my own branch and push it right? |
add test cases for function baseUrl
@davimacedo I believe I wrote a succesfull test. You can look it here And it passes every check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! LGTM!
@davimacedo Thank you. When will these changes be available on npm? |
I am not sure of the exact date, but I'm working to have a new release of Parse Server soon with this. |
* commit '73b3c780f0aca1011029012c13dd700accc577c1': Upgrade Parse to version 2.17.0 (parse-community#109) Replace greenkeeper badge with synk (parse-community#108) Update index.js (parse-community#106) Release v1.5.0 (parse-community#105) Update parse and aws-sdk to the latest versions (parse-community#104) Bump node-fetch from 2.6.0 to 2.6.1 (parse-community#103) fix: upgrade parse from 2.14.0 to 2.15.0 (parse-community#101) fix: upgrade aws-sdk from 2.706.0 to 2.709.0 (parse-community#98) fix: upgrade parse from 2.13.0 to 2.14.0 (parse-community#99) fix: upgrade aws-sdk from 2.705.0 to 2.706.0 (parse-community#96) Bump codecov from 3.6.5 to 3.7.1 (parse-community#97) fix: upgrade aws-sdk from 2.59.0 to 2.705.0 Bump lodash from 4.17.15 to 4.17.19 fix: upgrade parse from 2.10.0 to 2.13.0 (parse-community#93)
@@ -353,6 +353,49 @@ describe('S3Adapter tests', () => { | |||
expect(s3.getFileLocation(testConfig, 'test.png')).toEqual('https://myBucket.s3.amazonaws.com/foo/bar/test.png'); | |||
}); | |||
}); | |||
describe('getFileLocation', () => { | |||
const testConfig = { | |||
mount: 'http://my.server.com/parse', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not use invented domain names such as server.com
for testing purposes, that goes against RFC2606. IANA reserved example.*
domains for that purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I learnt something today!
Make baseUrl parameter function