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
fix: resourcePath in authorizers should contain wildcards #980
Conversation
Hi @danilofuchs thanks for your PR! "Some random tests break by timeout" --> I know, it sucks, did not write those. I tend to forget about them. |
Hi @danilofuchs, thanks for putting in this PR! Would you be able to resolve the conflicts before we merge? |
315e7d7
to
cdfdd92
Compare
Sure! I just rebased my branch and added a simple integration test. Thank you for taking my concerns into consideration. I now see it is much easier to test the repository |
cdfdd92
to
efc4866
Compare
Fixes #917
I still need to write tests for this fix.
I must say, the development experience to contribute to this project for the first time is quite poor.
The main issue is I could not run tests reliably in my machine:
old-unit
. I'm not confident in adding code to something withold
in its nameIn my opinion, tests should be easy to run locally, but not enforced to be ran on every push. Travis CI already does this tests for us, and is way more convenient. Running them locally on every push (which takes some minutes) will certainly make developers reconsider contributing ever again. For instance, Next.js is an immense project, and allows for flexible testing locally, and tests every PR on a CI.
If you would like, I can open an issue with my concerns so we can discuss it separately