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.

Sign up for GitHub

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

Jump to bottom

Fix computer launch for windows #635

Merged
merged 3 commits into from
Sep 22, 2021
Merged

Fix computer launch for windows #635

merged 3 commits into from
Sep 22, 2021

Conversation

yJunS
Copy link
Member

@yJunS yJunS commented Sep 20, 2021 •
edited
Loading

Make sure that you've checked the boxes below before you submit PR:

Always

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Written well with PR title, we generate the release notes base on that

For the bug fixes or features only

  • Quality Gate Passed. Change this URL to your PR.
  • The coverage is xxx on the new lines
  • I've tested it by manual in the following platform
    • MacOS
    • Linux
    • Windows
  • Unit Test covered
  • e2e Test covered

fix #634

@codecov
Copy link

codecov bot commented Sep 20, 2021 •
edited
Loading

Codecov Report

Merging #635 (6561121) into master (8f2c9f0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #635   +/-   ##
=======================================
  Coverage   37.98%   37.98%           
=======================================
  Files          13       13           
  Lines         416      416           
=======================================
  Hits          158      158           
  Misses        246      246           
  Partials       12       12
Flag Coverage Δ
unittests 37.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f2c9f0...6561121. Read the comment docs.

JohnNiang
JohnNiang suggested changes Sep 21, 2021
View reviewed changes
app/cmd/computer_launch.go Outdated
@@ -213,7 +215,14 @@ jcli agent launch agent-name --type jnlp`,
}

var f *os.File
if f, err = ioutil.TempFile("/tmp", "agent.jar"); err == nil {
var tmpPath = "/tmp"
Copy link
Member

Choose a reason for hiding this comment

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

IMO, we could use https://pkg.go.dev/io/[email protected]#TempDir to create a temporary directory here. And based on the temporary directory, we can create any files inside it.

@JohnNiang
Copy link
Member

hi @yJunS, please see the example: https://pkg.go.dev/io/[email protected]#example-TempDir. We don't need to specify /tmp and calculate temporary folder by hand.

@yJunS
Copy link
Member Author

yJunS commented Sep 21, 2021

hi @yJunS, please see the example: https://pkg.go.dev/io/[email protected]#example-TempDir. We don't need to specify /tmp and calculate temporary folder by hand.

Okay. I see. Maybe there is a certain problem with the old code usage

@JohnNiang
Copy link
Member

LGTM

@LinuxSuRen LinuxSuRen merged commit 05ca6b1 into jenkins-zh:master Sep 22, 2021
@LinuxSuRen LinuxSuRen added bug Something isn't working bugfix and removed bug Something isn't working labels Sep 22, 2021
jxr98 pushed a commit to jxr98/jenkins-cli that referenced this pull request Sep 27, 2021
@yJunS @jxr98
Fix computer launch for windows (jenkins-zh#635)
8f42115 
* Fix computer launch for windows

* Update create temporary directory

* Remove some code
jxr98 pushed a commit to jxr98/jenkins-cli that referenced this pull request Sep 28, 2021
@yJunS @jxr98
Fix computer launch for windows (jenkins-zh#635)
6e0789b 
* Fix computer launch for windows

* Update create temporary directory

* Remove some code
jxr98 pushed a commit to jxr98/jenkins-cli that referenced this pull request Sep 28, 2021
@yJunS @jxr98
Fix computer launch for windows (jenkins-zh#635)
6ff2781 
* Fix computer launch for windows

* Update create temporary directory

* Remove some code
LinuxSuRen added a commit that referenced this pull request Jun 15, 2022
@jxr98 @yJunS @LinuxSuRen
Add support conduct an API test of plugins (#624)
26b4f1f 
* ddd

ddd

update center_list

* add support to conduct api test for plugins

* fix some lint mistakes and update go.sum

* Fix computer launch for windows (#635)

* Fix computer launch for windows

* Update create temporary directory

* Remove some code

* Run release drafter in the GitHub action (#636)

* modify plugin_apitest.go

* resolve the conflict

* do some changes to pluginsWithProblemMap

* revert last 6 changes that include some commits from others

* do some changes to pluginsWithProblemMap

* modify go.mod and go.sum

* modify go.sum

Co-authored-by: Yanjun Shi <[email protected]>
Co-authored-by: Rick <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnNiang JohnNiang JohnNiang requested changes

@LinuxSuRen LinuxSuRen LinuxSuRen approved these changes

Assignees
No one assigned
Labels
bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

win10下使用jcli报错
3 participants
@yJunS @JohnNiang @LinuxSuRen