[JENKINS-31319] Retrieve method was implemented using GH API #4

recena · 2015-10-28T19:12:07Z

Console output example

Started by user anonymous
Connecting to GitHub Pull Requests using recena/****** (ec28dda8d2f4d4f59b8856ef56ed83d3e5106356)
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url https://github.com/seville-jam/website.git # timeout=10
Fetching upstream changes from https://github.com/seville-jam/website.git
 > git --version # timeout=10
using .gitcredentials to set credentials
 > git config --local credential.helper store --file=/tmp/git3051648955860258488.credentials # timeout=10
 > git fetch --tags --progress https://github.com/seville-jam/website.git +refs/pull/*/merge:refs/remotes/origin/pr/*
 > git config --local --remove-section credential # timeout=10
Checking out Revision 5c1b15276ab71c027000e1653466945a160d66f5 (PR-4)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 5c1b15276ab71c027000e1653466945a160d66f5
 > git rev-list 5c1b15276ab71c027000e1653466945a160d66f5 # timeout=10
[Workflow] Allocate node : Start
Running on master in /home/recena/projects/cloudbees-github-pull-requests-plugin/work/jobs/CJP-3204-4/branches/PR-4/workspace
[Workflow] node {
[Workflow] echo
Hello World 10
[Workflow] } //node
[Workflow] Allocate node : End
[Workflow] End of Workflow

GitHub has been notified of this commit’s build result

Finished: SUCCESS

@reviewbybees specially, @jglick

KostyaSha · 2015-10-28T19:18:31Z

CJP-3205

Can't login

recena · 2015-10-28T19:20:22Z

@KostyaSha I'm sorry, my mistake. I'm going to file a JIRA ticket.

KostyaSha · 2015-10-28T19:20:45Z

src/main/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubSCMSource.java

Please read @stephenc contributing guideline + checkstyle rules, they both against *. Every time you adding/removing classes imports will be collapsed to * or expanded to single that provides big useless changes in diff. i.e. here it is not obvious what was added.
PS in general there is no difference in performance because this imports influence only on build time AFAIK

@KostyaSha This plugin does not have that guideline. Anyway, I've changed it.

Yes, but after picking this rule i minimized my diffs :) it JFYI, you can ignore of course

KostyaSha · 2015-10-28T19:23:05Z

.gitignore

Suggesting install gitignore plugin if you are using IDEA or manually copy-paste IDEA, maven, java, eclipse, gradle (for future) ignores from https://github.com/github/gitignore

@KostyaSha The .gitignore already existed. I've added only two new entries.

You and new contributors will change gitignore every time and it will be infinitely. Better add all required at once ;)

should be *.iml

KostyaSha · 2015-10-28T19:27:43Z

Console output example

Is it a bug or fixed version?

recena · 2015-10-28T19:28:38Z

@KostyaSha Please, give me some minutes to fill the JIRA issue.

KostyaSha · 2015-10-28T19:29:54Z

src/main/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubSCMSource.java

Sorry what does this method do? If you are using github-plugin integration, then you don't need it's credentials as they should be set only globally.

If it about git, then github token can't work with git://

@KostyaSha This plugin manages its own credentials, you can find other example. Additionally, we are using only https because the GH API is exposed through https.

https://github.com/jenkinsci/github-branch-source-plugin/blob/master/src/main/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubSCMSource.java#L202-L210
This i fixed in github-api long time ago.

Using login+password for github is unsecure and wrong, but for hacky plugin will work.

@KostyaSha Why do you think it is using login+password and not login + apiToken?

Because in Connector i see standardUserCredentials

You can use that implementation for login + apiToken.

Token doesn't require login existence AFAIK

jglick · 2015-10-28T22:34:14Z

src/main/java/org/jenkinsci/plugins/github_branch_source/OriginGitHubSCMSource.java

🐛 you need to specify the actual commit hash here. Otherwise what would be the point?

jenkinsadmin · 2015-10-29T00:02:35Z

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

jglick · 2015-11-02T15:21:09Z

src/main/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubSCMSource.java

Not binary compatible, though I guess we do not care at this point.

We have only a beta release with 13 days 😵

And no coding styles for future trashing ;)

Actually we do care. This should fall back to a compatible behavior:

Selector selector = SCMHeadObserver.select(head); doRetrieve(selector, listener, repo); return selector.result();

jglick · 2015-11-02T15:25:15Z

🐝

jglick · 2015-11-02T19:12:45Z

Subsumed by #5. (Merging that would merge this automatically.)

[SECURITY-527] Enforcing USE_ITEM and @RequirePOST.

[JENKINS-57751] Fix config and code refractory

recena changed the title ~~[CJP-3205] Retreive method was implemented using GH API~~ [CJP-3205] Retrieve method was implemented using GH API Oct 28, 2015

recena force-pushed the CJP-3205 branch from d5966ae to bf349f4 Compare October 28, 2015 19:14

recena changed the title ~~[CJP-3205] Retrieve method was implemented using GH API~~ [WiP] Retrieve method was implemented using GH API Oct 28, 2015

KostyaSha reviewed Oct 28, 2015
View reviewed changes

Manuel Recena added 2 commits October 28, 2015 20:22

Retrieve method was implemented using GH API

700552a

Removed wildcard imports

689fa61

recena force-pushed the CJP-3205 branch from 9a16c96 to 689fa61 Compare October 28, 2015 19:23

KostyaSha reviewed Oct 28, 2015
View reviewed changes

jglick added the work-in-progress label Oct 28, 2015

jglick changed the title ~~[WiP] Retrieve method was implemented using GH API~~ Retrieve method was implemented using GH API Oct 28, 2015

jglick reviewed Oct 28, 2015
View reviewed changes

@jglick's comment has been addressed

a49806f

recena changed the title ~~Retrieve method was implemented using GH API~~ [JENKINS-31319] Retrieve method was implemented using GH API Oct 30, 2015

jglick reviewed Nov 2, 2015
View reviewed changes

jglick mentioned this pull request Nov 2, 2015

[JENKINS-31319] Implement retrieve(SCMHead, TaskListener) overload #5

Merged

jglick merged commit a49806f into jenkinsci:master Nov 2, 2015

recena deleted the CJP-3205 branch November 5, 2015 23:51

recena removed the work-in-progress label Nov 10, 2015

stephenc added a commit that referenced this pull request Jul 10, 2017

Merge pull request #4 from jenkinsci-cert/security-527-2.2.x

170ce22

[SECURITY-527] Enforcing USE_ITEM and @RequirePOST.

dubrsl pushed a commit to dubrsl/github-branch-source-plugin that referenced this pull request Oct 17, 2020

Merge pull request jenkinsci#4 from baymac/fix-config

fb928e6

[JENKINS-57751] Fix config and code refractory

jtnord mentioned this pull request Feb 23, 2021

Enable autoformatting #392

Merged

jenkins-infra-bot mentioned this pull request Jul 13, 2023

[JENKINS-44598] git workspace should be reset before build PR merged with base branch #1085

Open

jenkins-infra-bot mentioned this pull request Mar 4, 2022

[JENKINS-54610] Race condition when polling for changes triggered by web hook #1222

Closed

[JENKINS-31319] Retrieve method was implemented using GH API #4

[JENKINS-31319] Retrieve method was implemented using GH API #4

Uh oh!

Conversation

recena commented Oct 28, 2015

Console output example

Uh oh!

KostyaSha commented Oct 28, 2015

Uh oh!

recena commented Oct 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KostyaSha commented Oct 28, 2015

Uh oh!

recena commented Oct 28, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jenkinsadmin commented Oct 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglick commented Nov 2, 2015

Uh oh!

jglick commented Nov 2, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants