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

Improve developer experience #1736

Merged

Conversation

ThisIsMissEm
Copy link
Contributor

@ThisIsMissEm ThisIsMissEm commented Oct 2, 2024

Summary

Whilst working on #1735, I noticed it was hard to debug the code, this change adds a fairly opinionated way to debug the code when developing the doorkeeper gem. Additionally I've fixed the warning in the tests that I mentioned in #1732.

Now when developing in either the code or the specs, you can use the debugger keyword to jump into an interactive debugging session.

Other Information

Why include irb in the Gemfile? That's been recommended by the irb authors, due to irb bundled with ruby frequently being out of date with the state of irb development.

Copy link
Member

@nbulaj nbulaj left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nbulaj nbulaj merged commit ce8536a into doorkeeper-gem:main Oct 3, 2024
8 of 23 checks passed
@nbulaj
Copy link
Member

nbulaj commented Oct 3, 2024

Hm interesting that 1 test started to fail 🤔

@ThisIsMissEm ThisIsMissEm deleted the feat/improve-developer-experience branch October 3, 2024 09:53
@ThisIsMissEm
Copy link
Contributor Author

Hm interesting that 1 test started to fail 🤔

I think that test case may be flakey, have seen failures for it in other branches too

stanhu added a commit to stanhu/doorkeeper that referenced this pull request Oct 24, 2024
doorkeeper-gem#1736 broke tests for
Rails < 7.1 because these versions compared
`config.action_dispatch.show_exceptions == false`. Conditionally set
this value in the test set up to fix the broken tests.
@stanhu
Copy link
Contributor

stanhu commented Oct 24, 2024

This change broke tests for Rails < 7.1: #1742. I don't think this was flaky.

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.

3 participants