Skip to content
This repository has been archived by the owner on Aug 4, 2024. It is now read-only.

Add DataLine plugin #265

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add DataLine plugin #265

wants to merge 1 commit into from

Conversation

exodist
Copy link
Member

@exodist exodist commented Apr 28, 2023

No description provided.

@dboehmer
Copy link
Contributor

I do only see 1 added line in Changes. Did you add a .pm file and forgot to add it before committing?

@exodist
Copy link
Member Author

exodist commented Apr 28, 2023

oooof, I forgot to commit the files, and I have since wiped them out. I will re-implement it.

Copy link
Contributor

@dboehmer dboehmer left a comment

Choose a reason for hiding this comment

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

I didn’t run your code but looked at the diff only.

my $fd = $event->facet_data;

if (my $line = $.) {
my $fh = eval '$${^LAST_FH}' || do { # Added in 5.18, the do is fallback
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn’t this check the Perl version instead of string eval?


our $VERSION = '0.000154';

use B();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider a space to make this distinct from a function call?

Suggested change
use B();
use B ();

@@ -1,5 +1,7 @@
{{$NEXT}}

- Add Test2::Plugin::DebugOnFail
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this was the name of your previous attempt. I consider DataLine a better name.

$out = $msg;
};
warn "blah";
$out =~ m/<(.+)> line $line/ ? $1 : '?';
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this regex more specific? What if the test name contains line?

I don’t understand why there are <> in this regex. You don’t want to utilize //x for compatibility, right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants