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

Ideas #2

Closed
gravitystorm opened this issue Sep 7, 2016 · 8 comments
Closed

Ideas #2

gravitystorm opened this issue Sep 7, 2016 · 8 comments

Comments

@gravitystorm
Copy link
Owner

@gravitystorm gravitystorm commented Sep 7, 2016

I'm putting all my ideas for work on this here, so that it doesn't clutter up the openstreetmap issue tracker with bad ideas, or good ideas that I never get around to doing.

  • change @user to current_user
  • use Paranoid gem instead of controller-based checking for hidden? etc all the time
  • support multiple APIs simultaneously (so that I can refactor the routes)
  • use resources in routes as much as possible
  • use haml for html views (erb is useful but inelegant)
  • use a third-party authorisation system
  • use devise
  • change away from hash rockets (makes the code look old and unmaintained)
  • use factorygirl
  • use rspec
  • use annotate
  • use named path in urls rather than building them with hashes
@erictheise
Copy link

@erictheise erictheise commented Sep 7, 2016

I was pleasantly surprised to see your PR come through for FactoryGirl this morning, @gravitystorm, and everything on this list looks like a reasonable and welcome improvement.

In the past, I've sensed reluctance to update the "rails port" to be more contemporary or more "Railsy". If the changes you're proposing are likely to be accepted I'd be happy to help converting erb to haml (or Slim), rolling out annotate, converting =>, etc.

@tomhughes
Copy link

@tomhughes tomhughes commented Sep 7, 2016

Well I think the problem will all these things is that we all know they're a good idea, but we also know they're a massive job.

That means (a) people are reluctant to attack them and (b) we're reluctant to take partial patches along those lines because we fear the job will never be completed and we'll be left with a mish mash of different styles.

The question is, how do we get past those hurdles and get things moving...

@tomhughes
Copy link

@tomhughes tomhughes commented Sep 7, 2016

Oh of course as a perl user I am 100% convinced that hash rockets are the one true hash style™ ;-)

@erictheise
Copy link

@erictheise erictheise commented Sep 7, 2016

Some, e.g., annotate, are only massive in the sense that they cause many changes throughout the codebase. But they can be done quickly and in a single PR. Others, like a change in templating language or a shift to rspec, will take more time but, speaking from experience, constant attention & effort will get them done.

Implementing devise and route refactoring are the tasks that seem truly massive to me.

@tomhughes
Copy link

@tomhughes tomhughes commented Sep 7, 2016

Yes moving to devise was long on my priority list but always looked like a massive task whenever I considered it... Which is why I was quite please when I had the bright idea of using omniauth for the social logins without also using devise on the basis it was at least a halfway step.

Moving our password login to use omniauth might be another useful step towards devise.

@gravitystorm
Copy link
Owner Author

@gravitystorm gravitystorm commented Sep 7, 2016

The half-finished project is a legitimate concern. I usually take the "one bite at a time" approach to eating elephants, but it has drawbacks.

In current terms, rewriting all tests to use Factories is a huge chunk of work. Is it preferable to do this in small chunks? Or work on a branch for N months? I wouldn't like to diverge for so long, and I wouldn't like to review the enormous PR at the end.

Or are we happy with incremental changes so long as we're confident the transition will complete in a reasonable length of time? If so, what aspects would give us confidence? Multiple authors, perhaps?

@tomhughes
Copy link

@tomhughes tomhughes commented Sep 7, 2016

I agree that I think it makes sense to take things in pieces as much as possible, it's just good to have some sort of plan/commitment as a project to moving ahead to completion.

@gravitystorm
Copy link
Owner Author

@gravitystorm gravitystorm commented Nov 15, 2017

Closing this issue - most of the above has either been implemented, or is covered by other issues, or perhaps isn't a short term priority anyway :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.