Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Conversation

liuhaotian
Copy link
Contributor

  • Add diff id to Differential
  • Report diff id in post build summary
  • Correct unit test for revision id

It would be nice to provide differential id in Differential class. The
diff id is extracted from raw conduit json in the same way as revision
id, although it is also available from Jenkins params DIFF_ID.

Mean while, the differential test is updated to distinguish diff id
from revision id.

* Add diff id to Differential
* Report diff id in post build summary
* Correct unit test for revision id

It would be nice to provide differential id in Differential class. The
diff id is extracted from raw conduit json in the same way as revision
id, although it is also available from Jenkins params DIFF_ID.

Mean while, the differential test is updated to distinguish diff id
from revision id.
this.rawJSON = rawJSON;
}

public String getDIffID() {
Copy link
Contributor Author

@liuhaotian liuhaotian Dec 28, 2016

Choose a reason for hiding this comment

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

Should it be public String getID() { instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

getDIffID -> getDiffId

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjx It may be inconsistent with the surrounding code like getRevisionID though.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 91.323% when pulling 8c0c78c on liuhaotian:master into 6f56ac6 on uber:master.

Copy link
Contributor

@jjx jjx left a comment

Choose a reason for hiding this comment

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

Feel free to merge after addressing comments. Thanks!

this.rawJSON = rawJSON;
}

public String getDIffID() {
Copy link
Contributor

Choose a reason for hiding this comment

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

getDIffID -> getDiffId

@liuhaotian
Copy link
Contributor Author

@jjx @sectioneight would you help me merge the pr? Github think I am not authorized to do so.

@jjx jjx merged commit 81a7063 into uber-archive:master Dec 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants