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: comment with rspec-stderr #2330

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

runephilosof-abtion
Copy link
Member

@runephilosof-abtion runephilosof-abtion commented Sep 12, 2024

We should avoid introducing noisy warnings in our test output.
Often the warnings are added by Dependabot upgrades. This way we will notice when it happens.

Make sure that your repository has no output when you merge/cherry-pick this, to avoid adding noise to every PR.

We should avoid introducing noisy warnings in our test output.
Copy link

github-actions bot commented Sep 12, 2024

Rspec stderr output:

warning: parser/current is loading parser/ruby33, which recognizes 3.3.5-compliant syntax, but you are running 3.3.4.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

@runephilosof-abtion

This comment was marked as resolved.

@runephilosof-abtion
Copy link
Member Author

This will probably include the annotations from #2328 in the comment...

It looks like it is using stdout, so it won't be a problem.
https://github.com/Drieam/rspec-github/blob/075c231617113d9a05c80513dff17c7c7e27dbae/lib/rspec/github/formatter.rb

@runephilosof-abtion runephilosof-abtion marked this pull request as ready for review September 12, 2024 11:54
@runephilosof-abtion
Copy link
Member Author

Optimally, not only from the rspec step.

Also, it would be nice to fix that warning before merging this.

@runephilosof-abtion runephilosof-abtion added the SLA Implement this on our SLA projects label Sep 12, 2024
@runephilosof-abtion
Copy link
Member Author

Maybe I should make the step fail after posting, so that Dependabot PRs are not auto merged.

runephilosof-abtion added a commit that referenced this pull request Sep 17, 2024
This removes a deprecation warning about ruby and parser gem mismatch.

Not going to 3.3.5 because it prints a deprecation warning with the
current version of `pry` (fixed in `master`, waiting for a release).

Clean the deprecation warning for #2330
@runephilosof-abtion
Copy link
Member Author

Oops. All the steps are being run twice (push and pull_request), wasting resources.

@runephilosof-abtion
Copy link
Member Author

I will test it on another project first. Turning it into a draft.

@runephilosof-abtion runephilosof-abtion marked this pull request as draft September 17, 2024 07:43
Copy link
Contributor

@jeppester jeppester left a comment

Choose a reason for hiding this comment

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

This is a really interesting idea.
I do wonder if people will learn to ignore this message that same way as normal terminal output.

I wish there was a way to print only the additional error output added by the PR's branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SLA Implement this on our SLA projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants