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

add -tail to job log #588

Merged
merged 2 commits into from
Aug 12, 2021
Merged

add -tail to job log #588

merged 2 commits into from
Aug 12, 2021

Conversation

jxr98
Copy link
Contributor

@jxr98 jxr98 commented Jul 3, 2021

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

@linuxsuren-bot
Copy link

Welcome @jxr98! It looks like this is your first PR to jenkins-zh/jenkins-cli 🎉

@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2021

Codecov Report

Merging #588 (dc70f13) into master (c1846ac) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #588   +/-   ##
=======================================
  Coverage   38.46%   38.46%           
=======================================
  Files          13       13           
  Lines         286      286           
=======================================
  Hits          110      110           
  Misses        164      164           
  Partials       12       12           
Flag Coverage Δ
unittests 38.46% <ø> (ø)

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 c1846ac...dc70f13. Read the comment docs.

Copy link
Member

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. The calculation seems wrong. Please take a look at it.

➜  jenkins-cli git:(tail) ✗ ./jcli job log test -t 2
Current build number: 1
Current build url: http://139.198.3.176:30180/job/test/1/
Finished: SUCCESS
➜  jenkins-cli git:(tail) ✗ ./jcli job log test -t 3
Current build number: 1
Current build url: http://139.198.3.176:30180/job/test/1/
[Pipeline] End of Pipeline
Finished: SUCCESS

Please make sure that you have executed the command: make fmt before you submit.

@@ -34,6 +34,8 @@ func init() {
i18n.T("Watch the job logs"))
jobLogCmd.Flags().IntVarP(&jobLogOption.Interval, "interval", "i", 1,
i18n.T("Interval of watch"))
jobLogCmd.Flags().IntVarP(&jobLogOption.NumberOfLines,"tail","t",-1,
i18n.T("the last number of lines of the log"))
Copy link
Member

Choose a reason for hiding this comment

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

The first letter should be capital.

Comment on lines 60 to 66
numberOfLinesStr :=args[2]
numberOfLines, err:=strconv.Atoi(numberOfLinesStr)
if err!=nil||numberOfLines<=0 {
err =fmt.Errorf(err.Error(),"lines of job must be a positive integer instead of '%s'",numberOfLinesStr)
}else{
jobLogOption.NumberOfLines=numberOfLines
}
Copy link
Member

Choose a reason for hiding this comment

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

It's not necessary to parse the value of tail. The following code can do that.

jobLogCmd.Flags().IntVarP(&jobLogOption.NumberOfLines,"tail","t",-1, i18n.T("the last number of lines of the log"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All righty. I've changed that. The lines of 'Current build number' and 'Current build url' are not counted in the 'tail number', but there was a bug about the number of lines to be printed and it has been fixed. Thx.

Copy link
Member

@LinuxSuRen LinuxSuRen left a comment

Choose a reason for hiding this comment

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

It works well. Thanks!

➜  jenkins-cli git:(tail) jcli job log test -t 7
Current build number: 1
Current build url: http://139.198.3.176:30180/job/test/1/
[Pipeline] // stage
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline
Finished: SUCCESS

@LinuxSuRen LinuxSuRen merged commit 4d9bfa3 into jenkins-zh:master Aug 12, 2021
@jxr98
Copy link
Contributor Author

jxr98 commented Aug 12, 2021

You're welcome. Thx for reviewing my code.

jxr98 added a commit to jxr98/jenkins-cli that referenced this pull request Aug 29, 2021
jxr98 added a commit to jxr98/jenkins-cli that referenced this pull request Sep 4, 2021
@LinuxSuRen LinuxSuRen added the enhancement New feature or request label Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants