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

Add support for passing a block to start method #14

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

GustavoCaso
Copy link
Contributor

@GustavoCaso GustavoCaso commented May 16, 2023

Add a new module function execute(filename) into GvlTracing wrapping call of start and stop methods.

This is one of the tasks from #13

@GustavoCaso
Copy link
Contributor Author

If we do not like the execute method we could rename the C methods to _start and _stop and add simply wrapper in Ruby land.

module GvlTracing
  class << self

    private :_start
    private :_stop

    def start(filename)
      _start(filename)

      if block_given?
        yield
        _stop
      end
    end

    def stop
      _stop
    end
  end
end

@ivoanjo
Copy link
Owner

ivoanjo commented Jun 6, 2023

Looks good! In #12 indeed we already are wrapping start/stop with Ruby versions as you suggested -- do you mind rebasing on top of master and adding the block_given? to start directly as you suggested?

When a block is provided the start method, it  yields
and calls stop.
@GustavoCaso
Copy link
Contributor Author

@ivoanjo Done 🎉

@@ -37,6 +37,11 @@ class << self
def start(file)
_start(file)
@path = file

if block_given?
yield
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivoanjo we might wan to think about error handling 😄

Copy link
Owner

Choose a reason for hiding this comment

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

Right! I think an ensure block calling stop is probably the way to go

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 53a21ca

@GustavoCaso GustavoCaso changed the title Add execute method Add support for passing a block to start method Jun 8, 2023
README.adoc Outdated Show resolved Hide resolved
Fix typo .)
Copy link
Owner

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

Awesome, thanks 👍

@ivoanjo ivoanjo merged commit 8eaba2d into ivoanjo:master Jun 12, 2023
@ivoanjo
Copy link
Owner

ivoanjo commented Jul 1, 2023

Thanks @GustavoCaso for the contribution! I've (finally, lol, thanks for the patience!) released it as v1.3.0 :)

@GustavoCaso GustavoCaso deleted the add-execute-method branch July 21, 2023 15:36
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