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

Revert to using stderr.puts for start up message #1818

Merged
merged 1 commit into from Sep 26, 2022

Conversation

jkowens
Copy link
Member

@jkowens jkowens commented Sep 26, 2022

Fixes #1813.

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

❤️ Thanks for a swift fix! ❤️

Copy link
Member

@epergo epergo left a comment

Choose a reason for hiding this comment

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

Looks good! However I think is worth commenting that in this case the Rubocop change was wrong only because Sinatra has its own warn method defined https://github.com/sinatra/sinatra/blob/master/lib/sinatra/base.rb#L1775-L1777 that adds the from... line

We could reconsider the utility of such warn method and if we find it useful we could rename it so we don't mess with Ruby methods.

@olleolleolle
Copy link
Member

@epergo Good eye!

What if that supporting method got a new name, such as #warn_for_deprecation?

@jkowens
Copy link
Member Author

jkowens commented Sep 26, 2022

@olleolleolle I think that is a good idea. It looks like that method is only used one place right now, the public=(value) setter. I wish I had caught that earlier and removed that setter 😭

@jkowens jkowens force-pushed the fix-boot-message branch 2 times, most recently from ffc2680 to 8773621 Compare September 26, 2022 14:28
Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Nice!

@epergo
Copy link
Member

epergo commented Sep 26, 2022

What if that supporting method got a new name, such as #warn_for_deprecation?

That looks great 🚀

I wish I had caught that earlier and removed that setter

Right, we could have dropped it for Sinatra V3 but I think we can remove it in a minor release, no need to wait for a major for this.

@jkowens jkowens merged commit 6ad41b1 into sinatra:master Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The header in version 3 is less user friendly
3 participants