Skip to content
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

Add LSB Steganography function #80

Closed
wants to merge 3 commits into from
Closed

Conversation

auyer
Copy link

@auyer auyer commented Apr 27, 2019

This function is capable of Encoding and Decoding using LSB Steganography

Fixes #79

Signed-off-by: Rafael Passos [email protected]

This function is capable of Encoding and Decoding using LSB Steganography

Fixes openfaas#79

Signed-off-by: Rafael Passos <[email protected]>
@alexellis
Copy link
Member

I think I understand the function now, but I don't think the user journey is good enough in the Portal to merge this right now.

There are several other functions with complex inputs beyond a URL where the experience is poor.

We need a help text to be displayed in the UI, at the very least a link out to the originating repo's README. cc @burtonr

@auyer
Copy link
Author

auyer commented May 31, 2019

I agree. I plan to get some more work done to this function in the future ( I'm unavailable at the moment).

Besides changing the function, I see two ways that the Portal could be improved to make complex functions more available:

  • Set a "request template" in the portal. This could be configured in the yaml, and show up when the input box is empty or with a button.
  • A tool to upload and encode files into a json for image-related functions.

Shall we discuss this in Slack ?

@alexellis alexellis closed this Oct 21, 2019
@alexellis alexellis reopened this Oct 28, 2019
@derek derek bot added the no-dco label Oct 28, 2019
@derek
Copy link

derek bot commented Oct 28, 2019

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@alexellis
Copy link
Member

Having a clear-up on GitHub issues / PRs over 1 month old. Let's revisit as necessary. Thank you for the PR.

@derek
Copy link

derek bot commented Feb 16, 2020

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide.
Tip: if you only have one commit so far then run: git commit --amend --signoff and then git push --force.

@derek derek bot added the no-dco label Feb 16, 2020
@auyer
Copy link
Author

auyer commented Feb 16, 2020

Hi @alexellis , I just re-visited this function.
What I did:

  • re-tested everything ( including from the UI),
  • improved the support for URLs,
  • did some minor improvements to the code and docs,
  • a few usability improvements,
  • migrated from dep to go.mod (still vendoring the only lib used)

Its working fine, and its usable once the user reads the quite simple README.md (witch is necessary to use most functions anyway)
Since this function deals with files, using it in the web UI is not the best experience, but it works. Most people will use it from code or from a terminal.

@LucasRoesler
Copy link
Member

@auyer the latest master of your function is not passing CI, perhaps we want to get that fixed first
image

@auyer
Copy link
Author

auyer commented Feb 25, 2020

@auyer the latest master of your function is not passing CI, perhaps we want to get that fixed first
image

Wow I changed the job name and forgot to change it in the workflow.
I just fixed the CI.

@alexellis alexellis closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Submission Request, LSB Steganography function
3 participants