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 mutli-pipeline to Gitea #225

Merged
merged 6 commits into from
Jul 5, 2021

Conversation

ygbillet
Copy link
Contributor

@ygbillet ygbillet commented Jun 9, 2021

from https://woodpecker.laszlo.cloud/multi-pipeline/

Signed-off-by: Yves-Gaël BILLET <ygbillet@ifotec.com>
@ygbillet
Copy link
Contributor Author

ygbillet commented Jun 9, 2021

A proposition for adding multi-pipeline to Gitea.

remote/gitea/gitea.go Outdated Show resolved Hide resolved
Accept code format suggestion from @6543

Co-authored-by: 6543 <6543@obermui.de>
@laszlocph
Copy link
Member

This addresses #131 for Gitea

Anyone wants to test this?
CI passing, implementation looks sane.

@6543 6543 added the forge/gitea gitea forge related label Jun 17, 2021
@speatzle
Copy link
Contributor

This Works for me, though it took me a while to figure out i need to set the Pipeline Path to .drone (the folder) and that if i have the pipelines .drone/.drone.yaml and .drone/.test.yaml the .drone.yml pipeline will run but the ui only shows the .test.yml one for me, is this a bug you can confirm?

@laszlocph
Copy link
Member

i need to set the Pipeline Path to .drone (the folder) and that if i have the pipelines .drone/.drone.yaml and .drone/.test.yaml

correct

the .drone.yml pipeline will run but the ui only shows the .test.yml one for me, is this a bug you can confirm?

You mean you have the multipipes in .drone/ and also a .drone.yml in root?
Hm, not sure how that should work. Needs to be checked and documented.

@speatzle
Copy link
Contributor

You mean you have the multipipes in .drone/ and also a .drone.yml in root?

No i have the pipelines .drone/.drone.yaml and .drone/.test.yaml
The ui wouldn't show me the .drone/.drone.yaml one but it would still run.

@6543 6543 added this to the v0.14.0 milestone Jun 18, 2021
@speatzle
Copy link
Contributor

@laszlocph can you test whether a .drone/.drone.yml pipeline is also invisible in the UI when using github multi-pipeline?

@6543 6543 added the enhancement improve existing features label Jun 28, 2021
@6543 6543 changed the title Enhan: Add mutli-pipeline to Gitea Add mutli-pipeline to Gitea Jun 28, 2021
@ygbillet
Copy link
Contributor Author

ygbillet commented Jun 30, 2021

We need to reflect change in Dir() at gitea_oauth.go.

But we will have a code duplication.

We should probaby work in two steps

  1. First duplicate code in gitea_oauth.go
  2. Merge gitea.go and gitea_oauth.go (another pull/request)

@6543 : what do you think ?

Signed-off-by: Yves-Gaël BILLET <ygbillet@ifotec.com>
@6543
Copy link
Member

6543 commented Jun 30, 2021

At the moment we have lot of different code and differnt code styles in remote.

In the end I'd like to refactor it to have one Interface and a base implementation, each remote do extend&overwrite.

For example: giteas webhook response have its own defined struct in woodpecker and we use the gitea-SDK, so we should use the Structs of the SDK since its up to date and tested ...

@ygbillet
Copy link
Contributor Author

ygbillet commented Jul 5, 2021

OK.

Do we need more work for this PR (multipipeline to GITEA) ?
We use it at work and it will be good not to have to maintain our own version.

@speatzle
Copy link
Contributor

speatzle commented Jul 5, 2021

We use it at work and it will be good not to have to maintain our own version.

We have the same situation, would be nice to have it in the latest image. i can make a new issue for the invisible pipeline problem, i belive that is a general multipipeline issue anyway and not specific to gitea.

@6543
Copy link
Member

6543 commented Jul 5, 2021

I can merge it once I had time to give it a final "manual live" test

@6543
Copy link
Member

6543 commented Jul 5, 2021

code is fine, and test worked too - didn't had a look int this ui issue but created an issue -> #239

@ygbillet would be nice if you could provide more info or even better dig bit into it :)

@6543 6543 merged commit c699ea5 into woodpecker-ci:master Jul 5, 2021
@ygbillet
Copy link
Contributor Author

ygbillet commented Jul 6, 2021

Thanks !

Regarding #239, i will give it a try (i'm not the one who found this bug)

@UnlimitedCookies
Copy link
Contributor

I guess the documentation now needs to be amended accordingly. Currently, it still states the following:

NOTE: Feature is only available for Github repositories. Follow this issue to support further development.
https://woodpecker.laszlo.cloud/multi-pipeline/

@6543
Copy link
Member

6543 commented Aug 11, 2021

@UnlimitedCookies -> #254