-
Notifications
You must be signed in to change notification settings - Fork 286
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
ci(check-ci-skip): fix commitMessagesMetadata.forEach is not a function #3614
Comments
@petermetz just from looking at the title, I think it failed due to the github api being rate limited and thus the variable |
I ran it again @petermetz and it seems to work fine now. Most likely either a rate limit issue or github api endpoint intermittent issue (But yes, we will update the code so that it doesn't skip anyway) |
Primary Changes ---------------- 1. Added condition to commitMessagesMetadata to check if commit message is array or not. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Added condition to commitMessagesMetadata to check if commit message is array or not. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Added condition to commitMessagesMetadata to check if commit message is array or not. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Added condition to commitMessagesMetadata to check if commit message is array or not. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Added condition to commitMessagesMetadata to check if commit message is array or not. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
@jagpreetsinghsasan If it's intermittently failing due to rate limits then we either have to call it quits on this (which is acceptable to me, at least we tried) or refactor it in a way that it does not depend on the GitHub API (see my earlier idea about using the local git installation to determine the commit metadata). Reasoning/general thoughts: If the check is not stable/reliable due to rate limits, then it's going to block PRs by mistake (false negatives) and people will get confused about what to do or how to fix it and we'll just spend a lot of time explaining it to them and sending messages back and forth even for simple contributions. |
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
@petermetz as you said earlier, we are now moving ahead by checking this from the commits locally itself (after the actions/checkout is executed). In this way, we can straightaway remove the dependency on the api. Also as the rate limit is 1000/hr as specified here, it was most likely an intermittent issue from github side (which is also not relevant now as @zondervancalvez is now modifying the code to use the lcoal commits instead) |
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
Primary Changes ---------------- 1. Changed the method in getting the commit message from GitHub API to shell command to avoid the rate limits in calling the API. 2. Same goes for the author of commit message, we use shell command to fetch the username. Fixes hyperledger-cacti#3614 Signed-off-by: bado <[email protected]>
The new CI skip check job destroyed the CI completely. It crashes by default and then skips all the tests by default which allows merging pull requests without ANY test coverage.
@jagpreetsinghsasan Let's prioritize this as an urgent one please. The easiest and quickest fix I see for now is to just roll back the pull request entirely and put the task back in the backlog for the team.
https://github.com/hyperledger-cacti/cacti/actions/runs/11657574657/job/32455341799?pr=3604
The text was updated successfully, but these errors were encountered: