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

Make Scopes#+ and #& work against a non-Scopes object #868

Merged
merged 1 commit into from
Jan 29, 2018
Merged

Make Scopes#+ and #& work against a non-Scopes object #868

merged 1 commit into from
Jan 29, 2018

Conversation

knu
Copy link
Contributor

@knu knu commented Jul 13, 2016

The intention of Scopes#+ calling super is unclear to me, but I believe it is useful for the operators like + and & to work against an array, or any object that is convertible to Array.

end

it 'can get intersection with an array' do
scopes = Scopes.from_string('public admin') & %w(write admin)

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.


private

def other_array(other)
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename it to something like to_array? I think other_array is not descriptive name for this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@nbulaj
Copy link
Member

nbulaj commented Jan 28, 2018

Hi @knu . Can you please give an attention to the comments above ? (Hound CI and mine). Also can you please add a changelog to the NEWS.md file and rebase commits to squash them? Thanks for your work!

@knu
Copy link
Contributor Author

knu commented Jan 29, 2018

OK, but as for Hound CI I'm a bit at a loss because I just followed the styles of existing code.

end

it 'can get intersection with an array' do
scopes = Scopes.from_string('public admin') & %w(write admin)

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

expect(origin.to_s).to eq('public admin')
end

it 'can get intersection with an array' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

it 'does not change the existing object' do
origin = Scopes.from_string('public admin')
origin & Scopes.from_string('write admin')
expect(origin.to_s).to eq('public admin')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.


it 'does not change the existing object' do
origin = Scopes.from_string('public admin')
origin & Scopes.from_string('write admin')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

it 'does not change the existing object' do
origin = Scopes.from_string('public admin')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

it 'raises an error if cannot handle addition' do
expect do
Scopes.from_string('public') + 'admin'
end.to raise_error(NoMethodError)
end
end

describe '#&' do
it 'can get intersection with another scope object' do
scopes = Scopes.from_string('public admin') & Scopes.from_string('write admin')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Line is too long. [87/80]

it 'raises an error if cannot handle addition' do
expect do
Scopes.from_string('public') + 'admin'
end.to raise_error(NoMethodError)
end
end

describe '#&' do
it 'can get intersection with another scope object' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

it 'raises an error if cannot handle addition' do
expect do
Scopes.from_string('public') + 'admin'
end.to raise_error(NoMethodError)
end
end

describe '#&' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -70,13 +70,36 @@ module Doorkeeper::OAuth
expect(origin.to_s).to eq('public')
end

it 'can add an array to a scope object' do
scopes = Scopes.from_string('public') + ['admin']

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -70,13 +70,36 @@ module Doorkeeper::OAuth
expect(origin.to_s).to eq('public')
end

it 'can add an array to a scope object' do

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@nbulaj
Copy link
Member

nbulaj commented Jan 29, 2018

@knu, ok, not a big deal. Can you please add a changelog entry tyo NEWS.md and rebase your commits to squash them? Thanx!

@knu
Copy link
Contributor Author

knu commented Jan 29, 2018

Thanks, I will!

@nbulaj
Copy link
Member

nbulaj commented Jan 29, 2018

One more moment - can you squash your 2 last commits into 1 please? 🙏

@knu
Copy link
Contributor Author

knu commented Jan 29, 2018

Done.

@nbulaj nbulaj merged commit 60bfed6 into doorkeeper-gem:master Jan 29, 2018
@nbulaj
Copy link
Member

nbulaj commented Jan 29, 2018

Thank you @knu! 🙇

@knu knu deleted the scopes_and_array branch January 30, 2018 03:49
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.

3 participants