Skip to content

Conversation

@fcojfernandez
Copy link
Member

See JENKINS-57557. configuration-as-code is compatible with the last version of github-branch-sources. This PR adds an integration test to check possible future issues.

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@varyvol @MRamonLeon @alecharp @rsandell
@timja @Casz

@fcojfernandez fcojfernandez changed the title Jenkins 57557 [JENKINS-57557] Add integration test with github-branch-source Sep 20, 2019

private Option<Descriptor<T>> lookupDescriptor(String symbol, CNode config) {
return getDescriptors()
.filter(descriptor -> findByPreferredSymbol(descriptor, symbol) || findBySymbols(descriptor, symbol, config))
Copy link
Member

@jetersen jetersen Sep 21, 2019

Choose a reason for hiding this comment

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

Actually this is the correct formatting: https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/.idea/codeStyles/Project.xml#L89

So please, revert the revert 😆

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps not, we should take the styling fix in a separate PR :)

Copy link
Member Author

Choose a reason for hiding this comment

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

The format change was made automatically by IntelliJ. My vote is always to make format changes in separate PRs, but as your preference.

@varyvol
Copy link

varyvol commented Sep 23, 2019

@fcojfernandez I can see also github-branch-source-plugin #247. Does it make sense to repeat the test in both plugins? Which is the usual strategy?

@fcojfernandez
Copy link
Member Author

@varyvol AFAIK, there is no strategy. I made the decision to add both tests since there was none. This way we can check:

  • Future changes in github-branch-source-plugin are still compatible at least with configuration-as-code:1.30
  • Future changes in configuration-as-code are still working with github-branch-source-plugin

@fcojfernandez
Copy link
Member Author

Hi @timja @Casz

Is it missing something else from my side? Can this PR be merged?

@timja timja added the chore a PR that adds to maintenance - used by Release Drafter label Sep 24, 2019
@timja timja merged commit c85357a into jenkinsci:master Sep 24, 2019
@timja
Copy link
Member

timja commented Sep 24, 2019

Meant to reply yesterday saying I'll merge it today, sorry. Done now

@fcojfernandez
Copy link
Member Author

@timja thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore a PR that adds to maintenance - used by Release Drafter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants