Skip to content

Remove the last traces of the no longer existing cmd_l command #19171

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 1 commit into from
Oct 6, 2021

Conversation

choroba
Copy link
Contributor

@choroba choroba commented Oct 4, 2021

cmd_l was still mentioned in the documentation and comments. Also,
there was no test for the "v" command which would test #18900.

@choroba
Copy link
Contributor Author

choroba commented Oct 4, 2021

Should fix #19170

@choroba choroba changed the title Fix the "v" command in the debugger Remove the last traces of the no longer existing cmd_l command Oct 4, 2021
@choroba
Copy link
Contributor Author

choroba commented Oct 4, 2021

Sorry, used an old revision originally.

@jkeenan
Copy link
Contributor

jkeenan commented Oct 4, 2021

I got two test failures.

porting/cmp_version.t   (Wstat: 0 Tests: 6 Failed: 1)
  Failed test:  6
porting/exec-bit.t      (Wstat: 0 Tests: 135 Failed: 1)
  Failed test:  106

I know that you will have to increment $VERSION in lib/perl5db.pl to fix the first. Don't yet know how to fix the second. Can you look into this, and then run make test_porting before updating the p.r.?

Thank you very much.
Jim Keenan

@jkeenan jkeenan added the needs-work The pull request needs changes still label Oct 4, 2021
@choroba choroba force-pushed the debugger_v branch 2 times, most recently from e5ec01e to 1462be2 Compare October 4, 2021 23:18
@choroba
Copy link
Contributor Author

choroba commented Oct 4, 2021

Sorry for that, should be OK now. The second change was caused by my editor, it automatically set the execute bit on the .t file and I forgot to remove the change from the commit.

@jkeenan
Copy link
Contributor

jkeenan commented Oct 4, 2021

Unfortunately, I'm still getting one porting test failure.

===(    6791;1  0/?  6678/?  12/?  1/? )================================# Failed test 106 - tarball will chmod +x ../lib/perl5db.t at porting/exec-bit.t line 60
# Remove the exec bit or add '../lib/perl5db.t' to Porting/exec-bit.txt
porting/exec-bit.t ........ Failed 1/135 subtests                       

@choroba
Copy link
Contributor Author

choroba commented Oct 4, 2021

Oops, the bit was still set. Sorry. Force pushed again.

@jkeenan
Copy link
Contributor

jkeenan commented Oct 5, 2021

Okay, this looks good to me. @tonycoz, do you want to review it?

@jkeenan jkeenan removed the needs-work The pull request needs changes still label Oct 5, 2021
@tonycoz
Copy link
Contributor

tonycoz commented Oct 6, 2021

Also, there was no test for the "v" command which would test #18900.

There are tests for v around line 1187 added in cf70408 are these insufficient?

cmd_l was still mentioned in the documentation and comments.
@choroba
Copy link
Contributor Author

choroba commented Oct 6, 2021

You are right, the existing tests are sufficient. I overlooked them.
So, it reduces my PR to just a comment and doc fix.

@tonycoz tonycoz merged commit 88121dd into Perl:blead Oct 6, 2021
@choroba choroba deleted the debugger_v branch January 10, 2024 19:38
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.

3 participants