-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: uploading files in sas request #23
Conversation
Perhaps we should add prettier as a pre-commit hook? |
…6-session Conflicts: src/routes/index.ts src/utils/upload.ts
src/utils/file.ts
Outdated
@@ -24,3 +24,7 @@ export const generateUniqueFileName = (fileName: string, extension = '') => | |||
new Date().getTime(), | |||
extension | |||
].join('') | |||
|
|||
export const addExtensionIfNotFound = (value: string, extension: string) => { |
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.
Not sure that I understand the purpose of this utility.
And probably you wanted to test, if value ends with .
, becase it is possible to create file.file.txt
on unix systems.
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.
Ok good point, I will use regex. Purpose is that you don't need to provide extension in _program
parameter since official SASStoredProcess
does not require it.
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.
hmm - we should ALWAYS just add the ".sas" extension. No need for a utility.
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.
I found a good approach. I will also write the tests.
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.
I agree with @allanbowe and we should just add .sas
if needed
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 still need utility to check if extension not there, to be added
No description provided.