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

Support build from webhook #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gautamabhishek46
Copy link
Contributor

}

@Extension
public static class GhprbRootActionCrumbExclusion extends CrumbExclusion {
Copy link
Member

Choose a reason for hiding this comment

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

Class has not been renamed


public void doIndex(StaplerRequest req,
StaplerResponse resp) {
List<WorkflowMultiBranchProject> projects = Jenkins.get().getItems(WorkflowMultiBranchProject.class);
Copy link
Member

Choose a reason for hiding this comment

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

It won't work as you expect. Even if the action itself is not protected, this method should have permission checks inside

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you suggest something using which I can do the same thing and permissions are taken care of.

import java.util.logging.Logger;

/**
* @author Honza Brázdil jbrazdil@redhat.com
Copy link
Member

Choose a reason for hiding this comment

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

really?

* @author Honza Brázdil jbrazdil@redhat.com
*/
@Extension
public class YamlRootAction implements UnprotectedRootAction {
Copy link
Member

Choose a reason for hiding this comment

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

@Restricted(NoExternalUse.class)

// Hence triggering branch indexing of all projects, it will create jobs for new branches and pull requests and
// delete jobs of closed pull requests.
for(WorkflowMultiBranchProject project: projects){
if(project.getProjectFactory().getDescriptor().getDisplayName().equals(YamlRootAction.BUILD_CONFIGURATION_MODE)) {
Copy link
Member

Choose a reason for hiding this comment

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

🐛 Checking against display name is unreliable. It will start failing once the string is localized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can use something else here. I will take care of this.

@Extension
public class YamlRootAction implements UnprotectedRootAction {

static final String URL = "yamlhook";
Copy link
Member

Choose a reason for hiding this comment

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

maybe we need a more specific URL including artifactId

import java.util.List;

/**
* @author Abhishek Gautam gautamabhishek46

Choose a reason for hiding this comment

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

this author tag is only used here in your plugin - if you're not going to use it everywhere, than I think this shouldn't be here

}

@Extension
public static class YamlRootActionCrumbExclusion extends CrumbExclusion {

Choose a reason for hiding this comment

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

Can you please write a javadoc telling what is the purpose of this inner class?

@kwhetstone kwhetstone removed their request for review August 21, 2020 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants