Skip to content

Basic logger implementation #7

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

Merged
merged 2 commits into from
Jan 16, 2018

Conversation

valikos
Copy link
Contributor

@valikos valikos commented Jan 15, 2018

This is initial version of logger.
TODO: refactor default logger initialization.

@valikos valikos force-pushed the feature/ability-logger branch from 84e52b3 to 39a5951 Compare January 15, 2018 20:42
@valikos valikos mentioned this pull request Jan 15, 2018
@@ -8,6 +10,8 @@ module ClassMethods
DEFAULT_ROLE_NAME = :base
DEFAULT_ROLE_BLOCK = proc { true }

attr_accessor :logger
Copy link
Owner

Choose a reason for hiding this comment

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

Why getter in class methods? 🤔

Copy link
Owner

Choose a reason for hiding this comment

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

We have to use instance variable for this instead class methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case logger use class scope and methods

class Post::Abilities
  include Kan::Abilities

  register 'read' do |user, _|
    logger # => logger instance
    logger.info 'Something happened'
    true
  end
end

Need to think how to pass logger from ability instance to action.

Copy link
Owner

Choose a reason for hiding this comment

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

just use #instance_exec :)

class Abilities
  def initialize(rules)
    @rules = rules
  end

  def ability
    rule = @rules[:post]
    proc { |payload| instance_exec(payload, &rule) }
  end

  def logger
    'I am logger'
  end
end

rule = proc { |params| puts "logger: #{logger}; hello; #{params}" }
a = Abilities.new(post: rule)
  
p a.ability
a.ability.call(test: :test)

# => #<Proc:0x007f85c3a2eef0@proc.rb:7>
# => logger: I am logger; hello; {:test=>:test}

Copy link
Owner

@davydovanton davydovanton left a comment

Choose a reason for hiding this comment

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

So, let’s set instanse for logger, and after that merge.

About “global” logger: I have a idea how to change default values for each abilities and I want to play with it today :)

Thanks for your activity I really appreciate it 💯

Copy link
Owner

@davydovanton davydovanton left a comment

Choose a reason for hiding this comment

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

Thanks for you work, it's great.

Could you rebase your branch from master? I want to check specs (I added CI) ❤️

@davydovanton davydovanton self-assigned this Jan 15, 2018
@valikos valikos force-pushed the feature/ability-logger branch from 2664c20 to 4006e5f Compare January 16, 2018 07:33
@davydovanton
Copy link
Owner

Awesome work, thanks for quick response 🔥

@davydovanton davydovanton merged commit eae5d2f into davydovanton:master Jan 16, 2018
@valikos valikos deleted the feature/ability-logger branch January 16, 2018 09:31
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.

2 participants