-
-
Notifications
You must be signed in to change notification settings - Fork 284
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
[Jenkins-52906] add support for jcasc #168
Conversation
Writing more tests to fix coverage 😅😤😇 |
I can't put my review comments for some reason, got that error
|
@artkoshelev github had a datastore issue: https://status.github.com |
yeah, took me a while to figure out it's on github side not mine =) |
src/main/resources/hudson/plugins/jira/JiraFolderProperty/config.jelly
Outdated
Show resolved
Hide resolved
src/main/resources/hudson/plugins/jira/JiraGlobalConfiguration/config.jelly
Outdated
Show resolved
Hide resolved
This is where I can sit cry in a corner, rewriting tests to please reviewers 😅 No, I would rather rebase after merge of another PR and link to the commit view that pleases the reviewer. |
This is the compare window your looking for: https://github.com/casz/jira-plugin/compare/simpleConstructor...casz:JENKINS-52906 |
great job with this fix, where do we stand though? were the comments addressed? |
No, haven't had time and seems like the PRs are blocked by stats padding the coverage if the comments were to be addressed. |
@Casz @ewelinawilkosz @olamy @artkoshelev - any updates? We really need this. Thanks |
if #177 can get merged I will rework this PR 😄 |
Please take another look 😄 |
src/test/java/hudson/plugins/jira/JiraGlobalConfigurationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/hudson/plugins/jira/JiraGlobalConfigurationTest.java
Outdated
Show resolved
Hide resolved
I will fix the compilation error later — followed @artkoshelev review comment to remove test in front of the name. |
@olamy @artkoshelev I should have addressed all your concerns. |
instead of jira project property descriptor and stapler hacks aligning with JCasC fixup
Conflict resolved |
@olamy @artkoshelev what are the chances of this getting merged? |
It looks good to me, but change is pretty big, did you performed any kind of manual/integration testing? |
I don't actively use Jira. I am just here fixing the JCasC support plus some more while I was at it. perhaps @kmcquade or @timja can 😅 You should able to grab the HPI here: https://repo.jenkins-ci.org/incrementals/org/jenkins-ci/plugins/jira/3.0.7-rc971.e58465f3b1cd/ I wouldn't agree with big changes, most of it is done in testing. Very little is changed in the actual code besides some Java 8 cleanup. |
Removing all the test addition and import changes I am left with the significant change which are 152 additions and 99 deletions. So how big was this change? |
I did do some testing. Tested that configuration would be transferred to the new JiraGlobalConfiguration and tested that nested folder sites would show up. Did you not see the screenshots in the PR description? |
@olamy Would be good to get this in :) |
@olamy @artkoshelev users are asking for the jcasc support in our gitter channel |
JENKINS-52906
jenkinsci/configuration-as-code-plugin#809
Make URL the only mandatory field and simplify constructor. See Simplify constructor, main URL is the only mandatory field #173Use numbers in jira site's jelly. See use numbers on jira site jelly #172faster travis build & Use enable Jacoco flag. See speed up travis and use enable-jacoco profile #171This is easier to shown in UI, we now support folder in folder lookup of Jira Site 🙌
Hmm wonder if it would be better to sort the list 😅
Nice jcasc doc:
Nice jcasc export: