Closed openroadie closed 11 months ago
Please run a secure CI for this
clang-tidy review says "All clean, LGTM! :+1:"
All regressions in OpenROAD/test pass with the code in this PR
Please run a secure CI for this
Ok. So if I recall the process right:
Let me know if I missed anything.
Yes - in the private ORFS repo.
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
I'm not sure why it only shows up in mac but:
2023-06-15T18:28:47.7178460Z /Users/openroad/github-action-runners/actions-runner-1/_work/OpenROAD/OpenROAD/src/gui/src/chartsWidget.cpp:212:54: error: no member named 'scaleAbreviation' in 'sta::Unit'; did you mean 'scaleAbbreviation'?
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
clang-tidy review says "All clean, LGTM! :+1:"
Does this make sense in light of #3476 ? Wouldn't that include this as well?
I don't see a secure CI result for this. What branch name was used?
Yes. Don’t we have a policy of many small PRs. The previous PR is already one month old. So let’s merge that and then worry about this incremental change
On Sat, Jun 17, 2023 at 7:19 AM Matt Liberty @.***> wrote:
Does this make sense in light of #3476 https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476 ? Wouldn't that include this as well?
— Reply to this email directly, view it on GitHub https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595772670, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI . You are receiving this because you authored the thread.Message ID: @.***>
I will do do a secure ci. Let’s close on the other one which now has everything needed to merge. We don’t want to get caught with no upgrades should secure ci show us problems with this pr On Sat, Jun 17, 2023 at 7:21 AM Matt Liberty @.***> wrote:
I don't see a secure CI result for this. What branch name was used?
— Reply to this email directly, view it on GitHub https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595773140, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3MSTIZ7SCIEBCGJO6XXDRLXLW4QHANCNFSM6AAAAAAYD2JDTI . You are receiving this because you authored the thread.Message ID: @.***>
I don't see a secure ci. If you have one that's fine but if you have to start another it seems simpler to do it once. If you prefer to do them serially that's fine too.
On Sat, Jun 17, 2023 at 8:34 AM Harsh Vardhan @.***> wrote:
Yes. Don’t we have a policy of many small PRs. The previous PR is already one month old. So let’s merge that and then worry about this incremental change
On Sat, Jun 17, 2023 at 7:19 AM Matt Liberty @.***> wrote:
Does this make sense in light of #3476 https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476 https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476*3E__;JQ!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xkx2VK7Kg$ ? Wouldn't that include this as well?
— Reply to this email directly, view it on GitHub < https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595772670> https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595772670*3E__;IyU!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xnC3IbJrg$,
or unsubscribe < https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI> https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI*3E__;JQ!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xl1ruh1DA$ . You are receiving this because you authored the thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595789291__;Iw!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xl2ui_Ztw$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AAFZ5KWPNV3STWBJOWHFOO3XLXFB5ANCNFSM6AAAAAAYD2JDTI__;!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xlpihu4DQ$ . You are receiving this because you were mentioned.Message ID: @.***>
The original pr was held up because of secure ci and issues with routing etc or do you not remember that ? I will find out the branch name and post here if you cannot find it
On Sat, Jun 17, 2023 at 9:04 AM Matt Liberty @.***> wrote:
I don't see a secure ci. If you have one that's fine but if you have to start another it seems simpler to do it once. If you prefer to do them serially that's fine too.
On Sat, Jun 17, 2023 at 8:34 AM Harsh Vardhan @.***> wrote:
Yes. Don’t we have a policy of many small PRs. The previous PR is already one month old. So let’s merge that and then worry about this incremental change
On Sat, Jun 17, 2023 at 7:19 AM Matt Liberty @.***> wrote:
Does this make sense in light of #3476 https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476 < https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476*3E__;JQ!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xkx2VK7Kg$>
? Wouldn't
that include this as well?
— Reply to this email directly, view it on GitHub <
https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595772670>
or unsubscribe <
https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI>
. You are receiving this because you authored the thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub < https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595789291__;Iw!!Mih3wA!DMf12x_eD5TIW4R83JQHDK9gl32QyX-2OPgUvLruYNhe6IsveV5dGvY2d78YFqLlKki7TgSGGD2ILBPc7xl2ui_Ztw$>,
. You are receiving this because you were mentioned.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595794988, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3MSTI6Q6NKKHTKKUO5MIYDXLXISRANCNFSM6AAAAAAYD2JDTI . You are receiving this because you authored the thread.Message ID: @.***>
If I could find it I wouldn't have asked.
On Sat, Jun 17, 2023 at 9:07 AM Harsh Vardhan @.***> wrote:
The original pr was held up because of secure ci and issues with routing etc or do you not remember that ? I will find out the branch name and post here if you cannot find it
On Sat, Jun 17, 2023 at 9:04 AM Matt Liberty @.***> wrote:
I don't see a secure ci. If you have one that's fine but if you have to start another it seems simpler to do it once. If you prefer to do them serially that's fine too.
On Sat, Jun 17, 2023 at 8:34 AM Harsh Vardhan @.***> wrote:
Yes. Don’t we have a policy of many small PRs. The previous PR is already one month old. So let’s merge that and then worry about this incremental change
On Sat, Jun 17, 2023 at 7:19 AM Matt Liberty @.***> wrote:
Does this make sense in light of #3476 https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476 https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3476*3E__;JQ!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qmT_PiBkw$ <
? Wouldn't
that include this as well?
— Reply to this email directly, view it on GitHub <
https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595772670> https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595772670*3E__;IyU!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qkL0R2tgw$
<
or unsubscribe <
https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI> https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/A3MSTI2JC4BOS4E7T4GE3D3XLW4IHANCNFSM6AAAAAAYD2JDTI*3E__;JQ!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qnShQZUwQ$
<
. You are receiving this because you authored the thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub <
or unsubscribe <
. You are receiving this because you were mentioned.Message ID: @.***>
— Reply to this email directly, view it on GitHub < https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341#issuecomment-1595794988> https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595794988*3E__;IyU!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qlHcV_JLg$,
or unsubscribe < https://github.com/notifications/unsubscribe-auth/A3MSTI6Q6NKKHTKKUO5MIYDXLXISRANCNFSM6AAAAAAYD2JDTI> https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/A3MSTI6Q6NKKHTKKUO5MIYDXLXISRANCNFSM6AAAAAAYD2JDTI*3E__;JQ!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qlTC4H4vg$ . You are receiving this because you authored the thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/The-OpenROAD-Project/OpenROAD/pull/3341*issuecomment-1595795353__;Iw!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qkxVHKIOA$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AAFZ5KW6YAPOOG75OMPN7ZDXLXIZXANCNFSM6AAAAAAYD2JDTI__;!!Mih3wA!H221ils03Dufxre5GUYNjpGZCTBLBlm-7s2bShb3TrWfcOboDnYcES_VRp1sjiZXtYKptahBtqBzNzKD1qlxSqzKNg$ . You are receiving this because you were mentioned.Message ID: @.***>
The result is likely too old to be meaningful. I'll merge this and the next and we can deal with the fallout afterwards. The drt fix had to be reverted anyways so this will fail again.
clang-tidy review says "All clean, LGTM! :+1:"