Add config YML param to plugin for ability to set config per job

jenkinsci / allure-plugin

Allure Jenkins Plugin

https://plugins.jenkins.io/allure-jenkins-plugin/

Other

84 stars 65 forks source link

Add config YML param to plugin for ability to set config per job #204

Closed mikesalvia closed 5 years ago

mikesalvia commented 6 years ago

PR will add the ability to set (optionally) a config path for Allure.

This is helpful in turning on and off plugins in Allure when using the Jenkins plugin.

mikesalvia commented 6 years ago

@baev Not sure who should validate this PR but I know you had mentioned something like this in and it was mentioned in #184.

mikesalvia commented 6 years ago

Got this in Jenkins... When PR was built.

mikesalvia commented 6 years ago

test this please

mikesalvia commented 6 years ago

Odd it looks like it is now running properly. Looks like there are some warnings that could be addressed.

mikesalvia commented 6 years ago

Anyone available to review or approve this PR?

mikesalvia commented 6 years ago

Still no review... :cry: (goes and hides in corner)

baev commented 6 years ago

Summon @eroshenkoam

mikesalvia commented 6 years ago

Any traction on this?

mikesalvia commented 6 years ago

@baev Is there another person we can poke on this?

mikesalvia commented 5 years ago

Bump!

mikesalvia commented 5 years ago

@baev Bump!

mikesalvia commented 5 years ago

@eroshenkoam This is the ignored PR that allows for config.yml to be added in the Jenkins plugin... I guess it can be closed.

baev commented 5 years ago

@mikesalvia I really appreciate your work on this, and I beg your pardon for the delay.

In our team @eroshenkoam is responsible for the plugin, and I can't merge the change without his approval. I discussed this pull request with him several times and he doesn't like the provided changes because we are going to change plugin mechanism at some point (mystical upcoming Allure 3).

mikesalvia commented 5 years ago

@baev not your fault at all, I totally get it you can't merge against the maintainer's wishes, however, there was no comment at all then to see #221 it kind of got a bit frustrating to see the same work approved with no PR description or comments on why this PR was no good.

Also, it is in my opinion odd that there are no updates being done for Allure 2 because of what will happen in Allure 3. Even after Allure 3 is out people may want to (or need to) stick with Allure 2 and still have this option. Isn't that why there is version control?

eroshenkoam commented 5 years ago

@mikesalvia, let's make few changes here and I will merge it into master

eroshenkoam commented 5 years ago

just few little changes and we are ready to merge it

mikesalvia commented 5 years ago

@eroshenkoam I have made the suggested updates. I found a bunch of other locations using configYml.

I also want to apologize for my reaction, it came from confusion and frustration, though it is no excuse. I understand that maintaining repositories is not an easy task, and often times things fall through and there are a lot of things that need attention, thank you for the review and working through this with me. I am happy to be able to contribute to this project!

baev commented 5 years ago

@mikesalvia could you please rebase from master

mikesalvia commented 5 years ago

@baev Re-based and force pushed! Building now.

eroshenkoam commented 5 years ago

ty) 👍

eroshenkoam commented 5 years ago

I will release it today

timofeee commented 5 years ago

Is this not an optional field? Updated our plugin yesterday and since we didn't specify a file we're getting "java.io.IOException: Is a directory" cause of it.

eroshenkoam commented 5 years ago

@timofeee sry, I fix it tomorrow

NilsHasenbanck4758 commented 5 years ago

This just broke our jenkins. We will downgrade for now.

Deniszen commented 5 years ago

interim fix, adding an empty yml file to the project, and add path to him