alefragnani / vscode-jenkins-status

Jenkins Status Extension for Visual Studio Code
MIT License
27 stars 20 forks source link

Changes to allow multiple Jenkins Job configuration and other improvements in UX #17

Closed eramitmittal closed 4 years ago

eramitmittal commented 6 years ago

Hi,

This is my attempt to allow multiple Jenkins job configuration in your extension. Please feel free to reject the request or modify the code as you deem fit.

Amit

eramitmittal commented 6 years ago

I request you to only merge https://github.com/alefragnani/vscode-jenkins-status/pull/17/commits/4b8d1585d4a99f2dac779437ef66f68d311de12b.

At the moment I can't figure out how to remove rest of the commits from pull request.

alefragnani commented 6 years ago

Hi @eramitmittal ,

First of all, I'm sorry for taking so long to comment on your PR. It's been a busy period and I couldn't take a look.

I hope I could take test your PR in the next week, but taking a quick look at the source, I don't see any any issue. But I would like to ask you to comment how would become the .Jenkins file format, because just looking at the source code, I couldn't figure out.

Thanks for you PR 👍

eramitmittal commented 6 years ago

Hi,

The new format will look like this:

[{ "name": "Jenkins build job", "url": "http://127.0.0.1:8080/job/myproject", "username": "jenkins_user", "password": "jenkins_password_or_token" }, { "name": "Jenkins test job", "url": "http://127.0.0.1:8080/job/myprojectTests", "username": "jenkins_user", "password": "jenkins_password_or_token" } ]

Also note that I have made another enhancement. By clicking on the status bar icon for Jenkins job, it opens up the corresponding Jenkins page in browser. I thought this would be handy.

Since VS code status bar buttons allow only one command to be executed, I dropped support for 'Open Jenkins (Console)' command.

eramitmittal commented 4 years ago

Hi,

I re-synced my fork with latest changes in the original project and also added some new support (like support for .jenkisrc.js file, polling multiple jobs, on-the fly reflection of changes in .jenkins or .jenkinsrc.js file, status bar improvements etc.). I have re-done readme.md to explain what's different in the fork from your project.

Request you to re-evaluate the PR.

Note: I had to remove what's new as I was unable to make it work

Thanks, Amit

alefragnani commented 4 years ago

Hi @eramitmittal ,

I’m sorry for the silence in this PR. I was focusing on other projects, mostly because I’m not using Jenkins that much. Most of my CI has moved to GitLab, with just a few projects still using basic Jenkins jobs.

I’ll try to test your PR this week, and if everything works as expected, you could see a new release being published next weekend.

BTW, I guess your PR also closes #25, right?

Thank you

alefragnani commented 4 years ago

I just noted (tested) and #25 is not closed with your PR. No problem 👍

eramitmittal commented 4 years ago

Hi @alefragnani thanks for taking out time. It does close #25. Only things is that the changes appear on next poll.

Other option for the user is to manually invoke 'Jenkins: Update Status' command to see the changes immediately

It also allows to solve #33 (and #16 which is essentially the same issue i.e. discover current branch name automatically and point jenkins status plugin to it) as user can write ,jenkinsrc.js file to run arbitrary code while determining the jenkins config. For e.g.

function getJenkinsConfig() {
  const branchNameReader = require("current-git-branch");
  const branchName = branchNameReader({ altPath: __dirname });

  return [
    {
      name: "Jenkins Build",
      url: `http://jenkins/job/${branchName}/`
    }];
}

module.exports = getJenkinsConfig();
alefragnani commented 4 years ago

That’s great to see #33 would be closed with it. It’s a clever solution to import dynamic sources :-)

I’m not familiar with Multibranch pipelines in Jenkins (never used/needed) but I see your snippet would do the trick. I’m just wondering how the current-git-branch module would be handled by any project. I mean, it should be installed locally In each project/repo, right?

eramitmittal commented 4 years ago

yes, it is just an example. Whatever dependencies are required they must be present in node_modules directory where .jenkisrc.js executes.

Point is that dynamic configuration is possible through code. It is a strategy which a lot of tools use now a days including mocha, prettier etc.

alefragnani commented 4 years ago

I just found an issue about the newly added name attribute. If you don’t define it, the Status Bar will combine Jenkins with the number of no-named setting. That’s ok, and it is working fine In the Status Bar.

But, If you try to use the Open in Jenkins or Open in Jenkins (Console Output) commands, no PickList is displayed. This happens because it uses:

    vscode.window.showQuickPick(settings.map(setting.name)....

But name is undefined

I’ll change that, to display the generated name, or the URL. I’ll see which way works best.

No need to push any fix, because I’m already making updates in your fork (on my branch), restoring a few things from the master branch.

eramitmittal commented 4 years ago

Thanks, I missed that. Please let me know if I can help in anyway

alefragnani commented 4 years ago

I just fixed that :-)

I also noted that it was updating the status more frequently than before. I fixed that, removing the changes on onChangeActiveFile, and added a fileWatcher. This will close #25 .

The update will be released this weekend.

Stay tuned