[EWS] Link commit URL to pull request by JonWBedard · Pull Request #136 · WebKit/WebKit

JonWBedard · 2022-02-09T20:40:41Z

`e09ac79`

[EWS] Link commit URL to pull request
https://bugs.webkit.org/show_bug.cgi?id=236399
<rdar://problem/88707207 >

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/steps.py:
(GitHub.commit_url): We should link to commit within pull request
(ConfigureBuild.add_pr_details): Pass pr_number to GitHub.commit_url.
* Tools/CISupport/ews-build/steps_unittest.py:
(TestGitHub.test_pr_url_with_repository):
(TestGitHub):
(TestGitHub.test_pr_url_with_invalid_repository):
(TestGitHub.test_commit_url_with_repository):
(TestGitHub.test_commit_url_with_invalid_repository):


Canonical link: https://commits.webkit.org/247040@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@289506 268f45cc-cd09-0410-ab3c-d52691b4dbfc

aj062

Looks good. Can you please share a sample run as well?

aj062 · 2022-02-09T23:10:21Z

Thanks for adding these. Can also add unit-tests for cases when repository_url was provided (valid and invalid).

JonWBedard · 2022-02-09T23:47:08Z

Looks good. Can you please share a sample run as well?

Here is a good example run: https://ews-build.webkit-uat.org/#/builders/9/builds/3269

JonWBedard · 2022-02-09T23:49:29Z

Landed https://commits.webkit.org/247040@main (e09ac79)!

aj062 · 2022-02-10T01:10:54Z

Here is a good example run: https://ews-build.webkit-uat.org/#/builders/9/builds/3269

I wish I had this example before r+ing. This changes my opinion.
The link is not at all accessible once PR is landed, while the link without this change applied still works (e.g.: 3307e2c).

Earlier you mentioned in https://bugs.webkit.org/show_bug.cgi?id=235306#c1 that we wouldn't be able to have the commits displayed in the PR commits tabs after PR is merged. So, even though this url is better when PR is not merged, it's probably better to keep the previous behavior to generate hash urls which works even after PR is merged.

JonWBedard · 2022-02-10T01:58:27Z

Here is a good example run: https://ews-build.webkit-uat.org/#/builders/9/builds/3269

I wish I had seen this example before r+ing. This changes my opinion. The link is not at all accessible once PR is landed, while the link without this change applied still works (e.g.: 3307e2c).

Earlier you mentioned in https://bugs.webkit.org/show_bug.cgi?id=235306#c1 that we wouldn't be able to have the commits displayed in the PR commits tabs after PR is merged. So, even though this url is better when PR is not merged, it's probably better to keep the previous behavior to generate hash urls which works even after PR is merged.

That's unfortunate behavior, I will revert tomorrow. What's really surprising to me is that these commits seems to stay around as long as PRs reference them in some way, like this one from November: 6001db4

JonWBedard · 2022-02-10T15:58:12Z

Reverted in https://commits.webkit.org/247066@main (9942933)

https://bugs.webkit.org/show_bug.cgi?id=258794 Reviewed by Youenn Fablet. The current WebRTC HEVC is using generic packetization instead of RFC 7789 Packetization. So this is about implementing the RFC 7798 Packetization. Fix HEVC depacketizer issues. (WebKit#185) Enalbing low latency mode for RTC (WebKit#169) Enable HEVC support. (WebKit#165) Fix out-of-bounds write in H265VpsSpsPpsTracker (WebKit#163) Apply fix bitstream logic to RtpVideoStreamReceiver2 (WebKit#142) Add missing CODEC_H265 switch case (WebKit#136) Add HEVC support for iOS/Android (WebKit#68) H265 packetization_mode setting fix (WebKit#53) Add H.265 QP parsing logic (WebKit#47) This patch is extracted from following Open WebRTC Toolkit code changes: <open-webrtc-toolkit/owt-deps-webrtc#185> <open-webrtc-toolkit/owt-deps-webrtc#169> <open-webrtc-toolkit/owt-deps-webrtc#165> <open-webrtc-toolkit/owt-deps-webrtc#163> <open-webrtc-toolkit/owt-deps-webrtc#142> <open-webrtc-toolkit/owt-deps-webrtc#136> <open-webrtc-toolkit/owt-deps-webrtc#68> <open-webrtc-toolkit/owt-deps-webrtc#53> <open-webrtc-toolkit/owt-deps-webrtc#47> co-authoured by: taste1981 <[email protected]> jianjunz <[email protected]> Cyril Lashkevich <[email protected]> Piasy <[email protected]> ShiJinCheng <[email protected]> Andreas Unterhuber <[email protected]> dong-heun <[email protected]> * Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video/video_codec_type.h: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_codec.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_codec.h: (webrtc::VideoCodecH265::operator!= const): * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_decoder_software_fallback_wrapper.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_encoder.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/api/video_codecs/video_encoder.h: * Source/ThirdParty/libwebrtc/Source/webrtc/call/rtp_payload_params.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_common.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_common.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_pps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_pps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_sps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_sps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_vps_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/common_video/h265/h265_vps_parser.h: * Source/ThirdParty/libwebrtc/Source/webrtc/logging/rtc_event_log/rtc_event_log_parser.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/media/base/media_constants.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/media/base/media_constants.h: * Source/ThirdParty/libwebrtc/Source/webrtc/media/engine/internal_decoder_factory.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/create_video_rtp_depacketizer.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format_h265.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_format_h265.h: (webrtc::RtpPacketizerH265::PacketUnit::PacketUnit): (webrtc::VideoRtpDepacketizerH265::~VideoRtpDepacketizerH265): Deleted. * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/rtp_rtcp/source/rtp_video_header.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/codecs/h265/include/h265_globals.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/encoded_frame.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/h265_vps_sps_pps_tracker.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/h265_vps_sps_pps_tracker.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/include/video_codec_interface.h: * Source/ThirdParty/libwebrtc/Source/webrtc/modules/video_coding/packet_buffer.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/experiments/min_video_bitrate_experiment.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/nalu_rewriter.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/nalu_rewriter.h: * Source/ThirdParty/libwebrtc/Source/webrtc/test/scenario/video_stream.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/test/video_codec_settings.h: (webrtc::test::CodecSettings): * Source/ThirdParty/libwebrtc/Source/webrtc/video/config/video_encoder_config.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/config/video_encoder_config.h: * Source/ThirdParty/libwebrtc/Source/webrtc/video/encoder_overshoot_detector.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver2.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/rtp_video_stream_receiver2.h: * Source/ThirdParty/libwebrtc/Source/webrtc/video/send_statistics_proxy.cc: * Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_encoder.cc: * Source/ThirdParty/libwebrtc/libwebrtc.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/267677@main

JonWBedard self-assigned this Feb 9, 2022

JonWBedard requested a review from aj062 February 9, 2022 20:40

aj062 reviewed Feb 9, 2022

View reviewed changes

Comment thread Tools/CISupport/ews-build/steps.py Outdated

JonWBedard force-pushed the eng/github-ews-link-url-to-pr branch from dd6319a to 1dde3bb Compare February 9, 2022 22:52

aj062 approved these changes Feb 9, 2022

View reviewed changes

JonWBedard force-pushed the eng/github-ews-link-url-to-pr branch from 1dde3bb to 3307e2c Compare February 9, 2022 23:38

JonWBedard closed this Feb 9, 2022

JonWBedard force-pushed the eng/github-ews-link-url-to-pr branch from 3307e2c to e09ac79 Compare February 9, 2022 23:49

JonWBedard merged commit e09ac79 into WebKit:main Feb 9, 2022

JonWBedard deleted the eng/github-ews-link-url-to-pr branch February 9, 2022 23:50

JonWBedard added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Nightly Build labels Mar 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[EWS] Link commit URL to pull request#136

[EWS] Link commit URL to pull request#136
JonWBedard merged 0 commit into
WebKit:mainfrom
JonWBedard:eng/github-ews-link-url-to-pr

JonWBedard commented Feb 9, 2022 •

edited

Loading

Uh oh!

aj062 left a comment

Uh oh!

Uh oh!

aj062 Feb 9, 2022

Uh oh!

JonWBedard commented Feb 9, 2022

Uh oh!

JonWBedard commented Feb 9, 2022

Uh oh!

aj062 commented Feb 10, 2022 •

edited

Loading

Uh oh!

JonWBedard commented Feb 10, 2022

Uh oh!

JonWBedard commented Feb 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JonWBedard commented Feb 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

e09ac79

Uh oh!

aj062 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aj062 Feb 9, 2022

Choose a reason for hiding this comment

Uh oh!

JonWBedard commented Feb 9, 2022

Uh oh!

JonWBedard commented Feb 9, 2022

Uh oh!

aj062 commented Feb 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonWBedard commented Feb 10, 2022

Uh oh!

JonWBedard commented Feb 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JonWBedard commented Feb 9, 2022 •

edited

Loading

`e09ac79`

aj062 commented Feb 10, 2022 •

edited

Loading