Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Sep 14, 2016

➜  boxroom git:(master) ✗ rake test
Run options: --seed 56390

# Running:

.....................................................................

Finished in 2.220083s, 31.0799 runs/s, 145.4900 assertions/s.

69 runs, 323 assertions, 0 failures, 0 errors, 0 skips

Copy link
Owner

@mischa78 mischa78 left a comment

Choose a reason for hiding this comment

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

Hi, this looks good, thank you! I see some minor problems with the code style like using double quotes instead of single quotes and removing newlines at the end of the file. Can you please check my comments and adjust? Thank you very much.

.gitignore Outdated
log/*
tmp/*
uploads/*
.idea
Copy link
Owner

Choose a reason for hiding this comment

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

This probably does not apply to most people. Can you take it out?

def share_link_email(share_link)
@share_link = share_link
mail(:to => share_link.user.email, :reply_to => share_link.user.email, :bcc => share_link.emails, :subject => t(:share_link_email_subject, :email => share_link.user.email))
mail(:to => share_link.user.email, :reply_to => share_link.user.email, :bcc => share_link.emails, :subject => t(:share_link_email_subject, :email => share_link.user.email+"("+share_link.user.name+")"))
Copy link
Owner

Choose a reason for hiding this comment

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

I'd prefer it if you do it like this:

:email => "#{share_link.user.email} (#{share_link.user.name})"


def self.root
@root_folder ||= find_by_name_and_parent_id('Root folder', nil)
@root_folder ||= find_by_name_and_parent_id(Folder.human_attribute_name("folder.root_folder"), nil)
Copy link
Owner

@mischa78 mischa78 Oct 1, 2016

Choose a reason for hiding this comment

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

I prefer single quotes. Could you please change that in all the human_attribute_name calls?

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

--
Boxroom
http://boxroomapp.com/
<%= root_url %> No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

Don't remove the last newline please.

@ghost
Copy link
Author

ghost commented Oct 8, 2016

Now I have some updates according to your requirements and recommended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants