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 for issue with p4 ticket parsing #4581

Merged
merged 4 commits into from Feb 11, 2019
Merged

Fix for issue with p4 ticket parsing #4581

merged 4 commits into from Feb 11, 2019

Conversation

aisoard
Copy link
Contributor

@aisoard aisoard commented Jan 31, 2019

Proposed fix for P4 polling with ticket has incorrect parsing #4574.

@codecov
Copy link

codecov bot commented Jan 31, 2019 •
edited

Codecov Report

Merging #4581 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4581      +/-   ##
==========================================
+ Coverage   88.44%   88.45%   +<.01%     
==========================================
  Files         323      323              
  Lines       33658    33658              
==========================================
+ Hits        29768    29771       +3     
+ Misses       3890     3887       -3
Impacted Files Coverage Δ
master/buildbot/changes/p4poller.py 91.24% <100%> (+1.38%) ⬆️

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 012727c...42d347c. Read the comment docs.

@tardyp tardyp closed this Feb 1, 2019
@tardyp tardyp reopened this Feb 1, 2019
@aisoard
Copy link
Contributor Author

aisoard commented Feb 5, 2019

I just duplicated the test. I'm trying to write a negative test too (for failed identification) but I have no idea how to run the testsuite.

@aisoard aisoard force-pushed the master branch 3 times, most recently from 12f0b27 to 449202f Compare February 5, 2019 08:14
@aisoard
Copy link
Contributor Author

aisoard commented Feb 5, 2019

Rebased on top of current HEAD.

@p12tic
Copy link
Member

p12tic commented Feb 6, 2019 •
edited

@aisoard: Could you rebase on top of the current master again? During the release of v2.0 we broke the smokes test which is now fixed there. Thank you!

@p12tic p12tic removed the needs tests label Feb 6, 2019
@aisoard
Copy link
Contributor Author

aisoard commented Feb 7, 2019

Ok, I just rebased. Crossing fingers...

Alexandre Isoard and others added 4 commits February 8, 2019 17:02
Remove unnecessary execute permission
6498a48
@aisoard
Extract FakeTransport class
d158fbb
@aisoard
Fix issue with p4poller ticket parsing
0707d29 
In recent versions of Perforce CLI the output of `p4 login -p` changed.
The ticket number is still on the last line of output (was 3rd line, is
now 2nd line).
Add negative unit test to p4poller ticket parsing
42d347c 
FIXME: P4Poller should probably fail on ticket parsing failure.
@aisoard
Copy link
Contributor Author

aisoard commented Feb 9, 2019

@p12tic @seankelly Am I missing something?

@p12tic
Copy link
Member

p12tic commented Feb 9, 2019

I think you don't miss anything. @tardyp is the one who usually merges PRs, but there's several day latency most of the time.

@tardyp
Copy link
Member

tardyp commented Feb 11, 2019

@aisoard thanks for the fix and tests!

@tardyp tardyp merged commit dc9e434 into buildbot:master Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seankelly seankelly seankelly left review comments

@p12tic p12tic p12tic approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@aisoard @p12tic @tardyp @seankelly