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 compression level CLI option #84

Merged
merged 7 commits into from
Nov 4, 2019
Merged

Conversation

kakkoyun
Copy link
Contributor

@kakkoyun kakkoyun commented Oct 25, 2019

This PR adds compression level CLI option.
Also piggybacks a lot of refactorings. (sorry for lengthy PR)

Fixes #72

Proposed Changes

  • Add compression level CLI option
  • Logs compression level when debug enabled
  • Upgrade go 1.13
  • Refactor error messages by using new go errors
  • Introduce structured logging
  • Refactor Makefile
  • Upgrade linter, Golangci-lint
  • Automate usage doc creation
  • Update docs

Description

Users now can specify compression level.

Checklist

  • Read the CONTRIBUTING document.
  • Read the CODE OF CONDUCT document.
  • Add tests to cover changes.
  • Ensure your code follows the code style of this project.
  • Ensure CI and all other PR checks are green OR
    • Code compiles correctly.
    • Created tests which fail without the change (if possible).
    • All new and existing tests passed.
  • Add your changes to Unreleased section of CHANGELOG.
  • Improve and update the README (if necessary).
  • Ensure documentation is up-to-date. The same file will be updated in plugin index when your PR is accepted, so it will be available for end-users at http://plugins.drone.io.

Fixes meltwater#72

- Refactor Makefile
- Remove test script in favor of Makefile
- Automate usage generation
- Upgrade Golangci-lint

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Contributor Author

@limayekaiwalya It's a long but a good one. Please take your time.

@limayekaiwalya
Copy link
Collaborator

@kakkoyun , Looking at it. Sorry for the long response times. I am taking a bit longer to understand the existing code so I can provide better reviews :)

@kakkoyun
Copy link
Contributor Author

kakkoyun commented Nov 1, 2019

@limayekaiwalya Please tell me if you need anything to understand the existing codebase, happy to help.

Copy link
Collaborator

@limayekaiwalya limayekaiwalya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@limayekaiwalya
Copy link
Collaborator

@kakkoyun A questions out of curiosity: Why did we switch to fmt.Errorf from errors.Wrap ?

@kakkoyun
Copy link
Contributor Author

kakkoyun commented Nov 4, 2019

@limayekaiwalya The real change is about %w in fmt.Errorf which has introduced recently with 1.13 release of Go, now we can wrap standard errors with a context without a need of an external package. Further, since we don't actually utilize all the features from pkg/errors package, like stack capture etc., I thought we don't need any other library for that, hence I removed it.

@kakkoyun kakkoyun merged commit 50833a8 into meltwater:master Nov 4, 2019
@kakkoyun kakkoyun deleted the gzip_log branch November 4, 2019 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: log gzip compression ratio
2 participants