-
Notifications
You must be signed in to change notification settings - Fork 229
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 Thor warning due to missing :after option #252
base: master
Are you sure you want to change the base?
Conversation
I don't think you missed any specs, for the most part there haven't been any. Adding tests to all functionality is a major part of the Sorcery v1 rework.
I'm not really sure either yet. Would you be interested in doing some research to see if any other libraries have specs for their generators? Rails might be a good candidate for that. I'm still in the process of grokking the crypto and config, so it'll be a while before I get to reworking the generators for v1. |
I will try and find some time to do that :) Thanks for the feedback! |
At an absolute stand-still for now. Any help is welcome. |
My fellow wizards.. Please forgive me. I got so close. Ultimately I am too frustrated. I have to chill for a minute. If the solution is simple here, please do point it out. |
It's still going to be a bit before I'm to the generators unfortunately. I recently finished the Rails bootstrapping part (god that was a hurdle), but there's still the config refactor which will be pretty hefty, as well as some other less major refactoring. @Inkybro how about taking a break from this for now, and once I'm to the generators I can reach out to you and we can look at this together? |
@athix Even better, why don't you let me help you with something else? I'm happy to revisit this -- I just know when to cut my losses. |
@Inkybro Something that would be helpful is consolidating all the open issues and PRs, along with the old stuff from #32 into essentially one big wishlist of changes. It may save some time if those considerations are made up-front rather than after the refactor is complete, and as it is right now I'm knee deep in trying to grok the existing code. |
got u, bb |
Somehow I accidentally "liked" my own thing? Ok, no problem. I will do that but tonight I will relax because i worked all week!! |
Sounds good, thanks @Inkybro! Hope you have a relaxing afternoon. 👍 |
get some rest my friend |
a3d9a9d
to
4a49ffe
Compare
@athix I finally got some very basic generator specs working. Would love to hear your feedback, let me know what you think. Going to try and find some time next week to work on the other things we discussed earlier in this thread. |
I'm going to have to do some heavy lifting generator-wise for v1 regardless, so I'll just focus on adding generator tests and getting it all fixed up in v1 instead of trying to muddle through the v0 stuff. |
Hey, I was trying to mess around a little with Sorcery tonight on Rails 6.0.3.2, Ruby 2.7.1p83, and I kept noticing some odd warning message about a flag not being set when I tried to use the generators. I am pretty sure it was actually harmless, and my "fixes" here may ultimately not be necessary or desirable, but I was bored and wanted to see if I could get rid of the warning.
I tried with 0.15.0, as well as the master branch, and still get the warning:
File unchanged! The supplied flag value not found! app/models/user.rb
(see more below)It is worth mentioning that the line still did get added to the model, despite the misleading warning. I ended up tracing the warning message back to Thor, and it looks like it's expecting an
:after
option to let it know where to insert the content.I also accidentally entered my model name without the --model flag while testing the generator a few times and it was slightly annoying, so I also added the list of available submodules into the generator with a little validation and guidance for users. Other than that I just did a little re-arranging/re-factoring.
I really wasn't sure how one would go about writing specs for a generator. If anyone would like to point me in the right direction, I'd be willing to give it a shot. It looks like none of this code was really tested in the first place, unless I just totally missed it, and looks like the code hadn't been touched too much in a while. I did run the specs before I opened this PR, though.
Anyway, I was just bored. Feel free to throw this away if it's useless or way off base or anything.