Closed stardust66 closed 5 years ago
This looks really cool!
I wouldn't know how to test that this works. But this has been a big user-requested feature for some time, so it would be pretty awesome to be able to do this.
It certainly looks functional in your screenshots, too. 👍
I'd like to try to push this through soon, since we're so behind on edits. Looks like code climate is yelling, but other than that, this seems good to me.
I'm not sure why Code Climate doesn't like it. The page has nothing and pressing analyze hasn't done anything yet. Maybe we can trigger a rerun with another commit?
The "Analyze" button took me to a totally empty/blank page, if I wasn't logged in, but when I was logged in to CodeClimate with my GitHub credentials and clicked it again, it told me it had "requested a new analysis":
Hopefully that makes Code Climate happy. . .
Otherwise, I do think adding another commit could cause Code Climate to refresh.
(In situations like these I have tended to do "git commit --amend" and "git push -f", rather than create actual new commits, since dummy commits can look odd in the commit history.)
Okay, you can disregard my previous two comments.
I think this is just because your branch doesn't have the code climate stuff in it: https://github.com/stardust66/refugerestrooms/blob/user-edits/.travis.yml
Ways to fix:
Any of those should work, probably.
Okay, looks like this has been rebased now. Thanks @stardust66.
CodeClimate has analyzed this, and says the create
method in restrooms_controller.rb
should be simpler / easier to read.
So, looking into this, we can either try to refactor / break this up and somehow make it simpler, or rather decide it's okay as-is. I don't read/edit the ruby code very much, so other folks' input would be appreciated on this one. Otherwise, there are no remaining issues with CodeClimate or Travis.
The merge conflict here is really small, by the way:
-ActiveRecord::Schema.define(version: 2015_10_18_191859) do
+ActiveRecord::Schema.define(version: 20180107182858) do
Context
Summary of Changes
From #414:
This migration is fine if no edits were added prior to running it (and rails shouldn't let that happen). Someone more familiar with migrations should take a look though.
Checklist
Screenshots of Editing Process