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.

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 with errors returns success #218

Open
KaanOzkan opened this issue Feb 5, 2024 · 3 comments
Open

CI with errors returns success #218

KaanOzkan opened this issue Feb 5, 2024 · 3 comments
Labels
ci Change related to CI

Comments

@KaanOzkan
Copy link
Contributor

KaanOzkan commented Feb 5, 2024

Problem

https://github.com/Shopify/rbi-central/actions/runs/7702547117/job/20991065604#step:4:458

@KaanOzkan KaanOzkan added the ci Change related to CI label Feb 5, 2024
@Morriar
Copy link
Contributor

Morriar commented Feb 6, 2024

@andyw8 was this fixed with #216?

@andyw8
Copy link
Contributor

andyw8 commented Feb 6, 2024

No, I checked the logs for errors before merging though.

@Morriar
Copy link
Contributor

Morriar commented Feb 6, 2024

Looking at older builds, it seems to be working 🤔 I could only find https://github.com/Shopify/rbi-central/actions/runs/7702547117/job/20991065604#step:4:458 that failed yet was considered successful.

Looking at the logs, this is interesting:

Deprecation warning: Thor exit with status 0 on errors. To keep this behavior, you must define `exit_on_failure?` in `RBICentral::CLI::Main`
You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.

Yet, this option was definitely set before the changes:

rbi-central/gem/exe/repo

Lines 176 to 179 in db6227f

sig { returns(T::Boolean) }
def self.exit_on_failure?
true
end

But has been changed during the Rubocop changes:

rbi-central/gem/exe/repo

Lines 174 to 182 in ebe1c26

class << self
extend T::Sig
sig { returns(T::Boolean) }
def exit_on_failure?
true
end
end
end

I'm not sure how we could have the Thor warning in this case. @andyw8, is it possible that the branch was pushed with this instead (look at the self.)? This is an error I do often when performing this kind of change.

      class << self
        extend T::Sig

        sig { returns(T::Boolean) }
        def self.exit_on_failure?
          true
        end
      end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Change related to CI
Projects
None yet
Development

No branches or pull requests

3 participants