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

Refactor and CI #2

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Refactor and CI #2

wants to merge 16 commits into from

Conversation

obalunenko
Copy link

With this PR I add the following enhancements:

  • Follow Go community project layout https://github.com/golang-standards/project-layout
  • CI/CD support for automation code checks and delivery release artifacts (executables for all platforms that could be downloaded)
  • Add makefile automation
  • Helper scripts for compiling, linting, and releasing the application

PS more refactor in next PRs

@obalunenko
Copy link
Author

@eleby, Could you please review this PR? 🙏🏻

@eleby
Copy link
Owner

eleby commented Mar 11, 2021

@eleby, Could you please review this PR? 🙏🏻

Of course ! Thanks for the PR ! 😉

cmd/pixelizer/path.go Outdated Show resolved Hide resolved
NAME=pixelizer

SHELL := env DOCKER_REPO=$(DOCKER_REPO) $(SHELL)
DOCKER_REPO?=eu.gcr.io/melsoft-infra
Copy link
Owner

Choose a reason for hiding this comment

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

I don't know much about dockers and makefiles, but doesn't this mean we won't be able to compile if this docker repo is down someday ? Also, it seems this repo has the name of your company. I don't know if it is relevant to use your company's docker repo here.

Copy link
Owner

Choose a reason for hiding this comment

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

If you mark this as resolved please provide an answer about it so I can know if you will fix it

Copy link
Owner

@eleby eleby left a comment

Choose a reason for hiding this comment

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

You added lots of things I really don't know about;

Could you update the readme to write a doc about the functionnalities that were added ? It seems like the installation and compilation are completely different with this version, I am a little lost. I'd love to have time to learn about every tool you added and especially what they change in the program use but I don't unfortunately :/

Anyway, great job about all these changes. Can't wait for these informations and the answers to my comments !

@obalunenko
Copy link
Author

obalunenko commented Mar 16, 2021

You're always welcome;) Thanks for your review!
Sure, I'll update the readme
There are no changes in functionality. Only CI/CD - now users no need to install go and build themselves this tool. After you add the tag https://semver.org/ and push it to remote - CI will be triggered and build binaries for all os/arch and publish them on GitHub repo page under releases

You can check example of releases how they will be look)
All artifacts and release notes generated automatically on ci/cd
https://github.com/obalunenko/instadiff-cli/releases

@eleby
Copy link
Owner

eleby commented Mar 20, 2021

Okay ! So -sorry again for being a beginner in these aspects-, how does it work ?
If I understand correctly, you have to provide a version number and it creates the binaries to be listed on the Github release page. But where do you provide this version number exactly ?

I will wait for your explanations, readme update and fix on the docker package line. I like the idea of improved releases, I hope to understand everything better so I can merge the PR.

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.

2 participants