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
Lossless compression module #1603
base: main
Are you sure you want to change the base?
Conversation
Thanks for opening this pull request! |
I can't seem to understand how do I pass the input to the compress_images function. I think it needs a path to the image but i am getting input as an argument from the draw function so i don't get how to pass and process it. @publiclab/is-reviewers |
"name": "lossless-compression", | ||
"description": "Losslessly Compresses images", | ||
"input": {} | ||
} |
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.
add a reference to the docs
output: output, | ||
UI: UI | ||
}; | ||
}; |
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.
does this work? What do you get as an output after running this? @daemon1024
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 can't seem to insert the lossless-compression as a step. I can't seem to understand how to pass the input in the function. The parameter required there is path to the file, but the incoming data is a dataURI.
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.
https://hastebin.com/bakopiwino.coffeescript
This is the error I get when i run npm run test-all
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.
don't run the test right now. Tests are to be ran later when the module is complete.
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.
see from line 64 to line 81 of Module.js file of MinifyImage module inside src/modules/MinifyImage. There they have taken dirname/images as a file path and passed imagemin on it. @daemon1024
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.
@niravasher that part in Minify Module seems like to be used when the inBrowser options is false. They created that to run rest and stuff prolly.
@blurry-x-face @jywarren @harshkhandeparkar @publiclab/is-reviewers please help @daemon1024 . I have quizes going on. |
@niravasher Thanks a lot for taking your time out from your quizzes and helping me out. I have asked for help in gitter channel too. Hopefully someone else can help me out too :) |
No problem bro. Always love to help (although I am not an expert here 😄 ) |
Codecov Report
@@ Coverage Diff @@
## main #1603 +/- ##
==========================================
+ Coverage 55.11% 65.08% +9.96%
==========================================
Files 117 134 +17
Lines 2344 2761 +417
Branches 360 440 +80
==========================================
+ Hits 1292 1797 +505
+ Misses 1052 964 -88
|
@daemon1024 Converting this to a draft for now. Please feel free to make it to a normal PR if you get time to push the requested changes :) |
Fixes #1505
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
npm run test-all
@publiclab/is-reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!