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

Bug/jenkins 36273 pipeline search api organization support #307

Merged
merged 7 commits into from
Jun 30, 2016

Conversation

vivek
Copy link
Collaborator

@vivek vivek commented Jun 30, 2016

Related to issue # .

Summary of this pull request:
.
.
.

@reviewbybees

@vivek
Copy link
Collaborator Author

vivek commented Jun 30, 2016

@cliffmeyers pipeline search api with organization support as well as param to exclude flattening of certain types (folder/multi-branch etc.). Please review.

@ghost
Copy link

ghost commented Jun 30, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@michaelneale
Copy link
Member

🐝 - this doesn't fix things fully, but moves it a bit closer.

@@ -23,7 +23,7 @@ export class PipelineRecord extends Record({
weatherScore: 0,
}) {
isFolder() {
return this.numberOfFolders !== null;
return this._class === 'io.jenkins.blueocean.service.embedded.rest.PipelineFolderImpl';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@michaelneale this works, but maybe we should simply keep code something like, this.numberOfFolders !== null && totalNumberOfBranches == null with TODO to say replace by class/capability, because this discovery of whats what needs to be dealt with in a common way, for example sometimes it needs to go and call /classes API to discover what other classes it knows or what other capabilities it might have. I think Keith was playing with this thing for JUnit report stuff.

Copy link
Member

Choose a reason for hiding this comment

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

as discussed, lets use _class for now (other logic is hard to get right, surprisingly).

Copy link
Contributor

Choose a reason for hiding this comment

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

Using _class is way more explicit than the num/folders/branches stuff which I always felt was a little too loose, so 🐝 - long term maybe referring to some kind of capability array with fixed values is a fine idea too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cliffmeyers @michaelneale This is fine as quick solution. However for UI to show any multi-branch, I mean a developer has its own implementation of multi-branch, how does that show up in the UI? For that, UI should really be checking against interface, in this case: io.jenkins.blueocean.rest.model.BlueMultiBranchPipeline. That is UI core knows about well known interfaces it supports and on receiving a _class, it should call a common utility class that calls /rest/classes/:classId API to get list of other supported classes/capabilities. Something like: supportsCapability(givenClass, expectedClass), returns boolean. See Class of resource discovery

So a TODO should be marked or perhaps a ticket to track this.

Copy link
Member

Choose a reason for hiding this comment

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

@vivek at the moment this is explicitly for filtering out what we know is vanilla folders, vs positively identifying things based on capabilities, so I think it will be ok for this as it won't stop new types showing up, just identify them by folders.

I believe we have some tickets open for capabilities, if not, we should open them for sure.

@michaelneale michaelneale merged commit af1d01e into master Jun 30, 2016
@michaelneale michaelneale deleted the bug/JENKINS-36273 branch June 30, 2016 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants