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

[WIP] Replace angularjs-rails dependency with rails-assets-* #3773

Closed
wants to merge 2 commits into from
Closed

[WIP] Replace angularjs-rails dependency with rails-assets-* #3773

wants to merge 2 commits into from

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Aug 9, 2015

This is to replace (ideally) all client-side dependencies with rails-assets-* versions.
I've moved them to a separate file - Gemfile.client.rb - to keep them separated.

Current TODO:

gem 'rails-assets-angular-route', '~> 1.4.3'
gem 'rails-assets-angular-sanitize', '~> 1.4.3'
gem 'rails-assets-angular-scenario', '~> 1.4.3'
gem 'rails-assets-angular-touch', '~> 1.4.3'
Copy link
Member

Choose a reason for hiding this comment

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

Interesting...do we actually use all this stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really, it's just all the stuff angularjs-rails adds. It's just that a quick search didn't show me mocks which I know we use, so I want to go through them properly to be sure

…ssets

Fixed the low-hanging fruit, now only the big stuff.
gem 'codemirror-rails', "=4.2"
gem 'lodash-rails', '~> 3.10.0'

# On MS Windows run "bundle config --local build.libv8 --with-system-v8" first
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@djberg96 Do you remember which dependency this line was supposed to be about please? (From 984d7ae)

Copy link
Contributor

Choose a reason for hiding this comment

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

@himdel, libv8. We've dropped that, though, right? If so, you can remove that comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it's not in any Gemfile I can see, so removing :)

@miq-bot
Copy link
Member

miq-bot commented Aug 18, 2015

@miq-bot
Copy link
Member

miq-bot commented Aug 18, 2015

Checked commits https://github.com/himdel/manageiq/commit/a41b908c90dc630ecda8e4c1e3f25b7d655c139a .. https://github.com/himdel/manageiq/commit/c290cb9e029c1ff440dacd2abbc537d709afd373 with rubocop 0.32.1 and haml-lint 0.13.0
2 files checked, 6 offenses detected

Gemfile.client.rb

  • 🔶 - Line 1, Col 1 - Style/FileName - Use snake_case for source file names.
  • 🔶 - Line 3, Col 3 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.
  • 🔶 - Line 5, Col 3 - Style/CommentIndentation - Incorrect indentation detected (column 2 instead of 0).
  • 🔶 - Line 5, Col 5 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.
  • 🔶 - Line 10, Col 3 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.
  • 🔶 - Line 28, Col 3 - Style/CommentAnnotation - Annotation keywords like TODO should be all upper case, followed by a colon, and a space, then a note describing the problem.

@miq-bot
Copy link
Member

miq-bot commented Aug 19, 2015

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@dclarizio dclarizio added the ui label Jan 19, 2016
@himdel
Copy link
Contributor Author

himdel commented Jan 29, 2016

closing in favor of whatever replaces #5640

@himdel himdel closed this Jan 29, 2016
@himdel himdel deleted the rails-assets-angular branch January 29, 2016 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants