-
Notifications
You must be signed in to change notification settings - Fork 71
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
Cleaning up and restructuring - Part 1 #90
Comments
This is already looking very good. 👍 I hope the code will not become significantly harder to read. Only change request: please use |
Merged
srevinsaju
referenced
this issue
in srevinsaju/go-appimage
Nov 3, 2020
probonopd
referenced
this issue
Nov 8, 2020
* fix: show verbose ELF headers only in case of errors * style: Add spacing around concat operator * style: remove redundant parenthesis * fix: remove verbose logging to stdout elf sections * feat: add helper function to determine if File or Folder exists * feat: provide overwriteSecretFiles as an argument to function and remove flag dependency The overwritePtr flag was defined globally which created conflicts when calling functions within the setupsigning.go from other functions i.e not through the command line. Poroviding an boolean argument solves the problem * feat: add a new dependency 'cli' instead of 'flag' 'cli' is a better module which helps to make the command line interfaces more neater and well structured, both functionally as well as visually. this would help * users to understand the tool, as its style syncs with other popular cli * developers, very nice functional interface, best for programmers and new contributors who are not familiar with the flag command A pre-exisiting module, which is in its v2 is better than a self implemented command line parser / low level go module . * style: define constants globally * feat: provide options DeployOptions to pass build instructions from cli to functions * feat: restructure and rewrite appimagetool.go, use urfave/cli/v2 * chore: update go dependencies * fix: do not pass options through all functions instead declare a single options->DeployOptions in appimagetool.go prior to calling AppImageBuild() * docs: add documentation for functions * fix: use log.Fatal instead os.Write.StdErr as log.Fatal has built in Error handling and automatically exits with status 1 * style: explicitly ignore errors in removing and chmod'ing the file * fix: returned err variable was not received. The check was done for the previous step instead * style: use named arguments * fix: use lower case in appimagetool name in cli https://github.com/probonopd/go-appimage/issues/90\#issuecomment-720104568
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I am restucturing
go-appimagetool
following #87 (comment) to get a better structure. This is the command line interface so far which was accompanied with the restructuring. Let me know of your ideas. comments so that I can incorporate in the PR.It looks a bit more neater for me, but this is followed by a lot of other changes too.
cc @probonopd
The text was updated successfully, but these errors were encountered: