Skip to content
This repository has been archived by the owner on Jun 21, 2020. It is now read-only.

WIP: Update bintray plugin format for 1.x.x compatibility #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arcolife
Copy link

@arcolife arcolife commented Dec 16, 2019

Add following for new bintray dep:

- json -> yaml: add yaml based config parser
- add forked bintray go client as dep
- godotenv mod to parse configs from .env / env vars all the same

Removed artifactory and will re-add once the above workflow is validated. For this very reason, I'm using a fork of: https://github.com/jfrog/jfrog-client-go -> github.com/arcolife/jfrog-client-go
The fork is waiting to be merged into master at jfrog/jfrog-client-go#103

@tboerger @bradrydzewski A rather delayed follow up from our discussion on gitter earlier at https://gitter.im/drone/plugins?at=5d7fd98572fe125111b77576 😄

TODO:

    - json -> yaml: add yaml based config parser
    - add forked bintray go client as dep
    - godotenv mod to parse configs from .env / env vars all the same
    - update documentation
Copy link
Contributor

@tboerger tboerger left a comment

Choose a reason for hiding this comment

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

I haven't reviewed reviewed now,but at least that's a starting point

@@ -0,0 +1,8 @@
BINTRAY_KEY=f4g8ef@***223
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only temporary for testing, right?

Copy link
Author

Choose a reason for hiding this comment

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

yep

@@ -2,3 +2,6 @@ release/

coverage.out
drone-bintray

*~
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like editor specific ignores, you should add that to your local global gitignore.

Copy link
Author

Choose a reason for hiding this comment

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

noted.

@@ -1,7 +1,12 @@
module github.com/drone-plugins/drone-bintray
module github.com/arcolife/drone-bintray
Copy link
Contributor

Choose a reason for hiding this comment

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

That should be drone-plugins

Copy link
Author

Choose a reason for hiding this comment

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

noted

&cli.IntFlag{
Name: "bintray.threads,t",
Usage: "bintray threads",
EnvVars: []string{"PLUGIN_BINTRAY_THREADS", "BINTRAY_THREADS"},
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is the bintray plugin please name them PLUGIN_ instead of PLUGIN_BINTRAY_

Copy link
Author

Choose a reason for hiding this comment

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

noted

}

func (config *BintrayConfig) deleteRepo(btManager *bintray.ServicesManager, repo *Repo) error {
fmt.Printf("\nDeleting Repo.. [%s]\n", repo.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

With all these log messages you should use a structured logger like logrus, we will anyway use it more within plugins as it's part of our upcoming plugin lib

Copy link
Author

Choose a reason for hiding this comment

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

right for some reason logger built in jfrog go library of bintray wasn't working. I was hoping to reuse that tho

// files []string
// )
// fmt.Printf("Drone Bintray Plugin built from %s\n", p.Version)
cSig := make(chan os.Signal)
Copy link
Contributor

Choose a reason for hiding this comment

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

We aren't doing that for any plugin and I can't clearly see why we should introduce that here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants