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

⚠️ IGNORE: Remove duplicate [GOTO: PR 21059] #21057

Closed
wants to merge 11 commits into from
Closed

⚠️ IGNORE: Remove duplicate [GOTO: PR 21059] #21057

wants to merge 11 commits into from

Conversation

rwp0
Copy link
Contributor

@rwp0 rwp0 commented Apr 29, 2023

▶️ GO TO: #21059

⚠️ And ignore this PR

@jkeenan
Copy link
Contributor

jkeenan commented Apr 29, 2023

  1. Changes to cpan/Compress-Raw-Zlib/zlib-src/zlib.have to be submitted upstream on CPAN.
  2. You have two porting test failures; can you fix and re-push?

@rwp0
Copy link
Contributor Author

rwp0 commented Apr 29, 2023

  1. Changes to cpan/Compress-Raw-Zlib/zlib-src/zlib.have to be submitted upstream on CPAN.

Thanks, I've amended my commit to revert the zlib.h case.

Created a separate PR for that purpose.

  1. You have two porting test failures; can you fix and re-push?

Unfortunately no, not alone by myself at least 🙁

@rwp0 rwp0 changed the title Remove duplicate "the" from comments Remove duplicate "the" in comments Apr 29, 2023
@jkeenan
Copy link
Contributor

jkeenan commented Apr 29, 2023

  1. Changes to cpan/Compress-Raw-Zlib/zlib-src/zlib.have to be submitted upstream on CPAN.

Thanks, I've amended my commit to revert the zlib.h case.

Created a separate PR for that purpose:

* [[doc] zlib.h: Remove duplicate "the" pmqs/Compress-Raw-Zlib#22](https://github.com/pmqs/Compress-Raw-Zlib/pull/22)

Moving that change out of the pull request magically cleared up one of your porting test failures. You had omitted incrementing $VERSION in a .pmfile, which caused the failure in t/porting/cmp_version.t.

  1. You have two porting test failures; can you fix and re-push?

Unfortunately no, not alone by myself at least

Well, figuring out how to deal with these failures is a useful skill for a regular committer. You would do well to run make test_porting on your local machine before pushing a pull request. But let's look at the failure output at https://github.com/Perl/perl5/actions/runs/4840634578/jobs/8626407229?pr=21057. You have 3 separate failures but all within one single test file, t/porting/regen.t. Two of those failures mention regen/mk_invlists.pl, which is also noted to require being run with the perl you've just built. So try running:

./perl -Ilib/regen/mk_invlists.pl

Examine files which have changed, run make test_porting again to see what has been fixed and what hasn't. Commit as needed.

The remaining test failure also involves generated code. Note the error output and look through the regen/ directory to spot a program that might clear that up. Then, examine files which have changed, etc.

@jkeenan
Copy link
Contributor

jkeenan commented Apr 29, 2023

@rjbs, assuming the content of this p.r. is approved and we get all the porting tests to PASS, is it okay to push this p.r. to blead (given code freeze)?

@rwp0
Copy link
Contributor Author

rwp0 commented Apr 29, 2023

Running ./perl -Ilib ./regen/mk_invlist.pl fixed the two failing tests below

porting/regen.t ........... 1/44 # Failed test 6 - generated charclass_invlists.h is up to date at porting/regen.t line 122
#      got "regen/mk_invlists.pl"
# expected ""
# Note: regen/mk_invlists.pl must be run manually, because it needs the Perl you've just built
# Failed test 7 - generated uni_keywords.h is up to date at porting/regen.t line 122
#      got "regen/mk_invlists.pl"
# expected ""

Output

Changed: charclass_invlists.h lib/unicore/uni_keywords.pl uni_keywords.h

And running ./perl -Ilib ./regen/uconfig_h.pl fixed the third failing test in regen.t:

# Failed test 8 - generated uconfig.h is up to date at porting/regen.t line 122
#      got "config_h.SH"
# expected ""

Output

 Extracting uconfig.h-new (with variable substitutions)
 Changed: uconfig.h

Thanks to GitHub Codespaces all tests PASS now

pass


Next

Will try to amend my commit to include these changes.

Fix spelling on various files pertaining to core Perl.
@rwp0 rwp0 marked this pull request as draft April 29, 2023 23:59
@rwp0 rwp0 closed this Apr 30, 2023
@rwp0 rwp0 changed the title Remove duplicate "the" in comments IGNORE: Remove duplicate "the" in comments [GOTO PR 21059] Apr 30, 2023
@rwp0 rwp0 changed the title IGNORE: Remove duplicate "the" in comments [GOTO PR 21059] IGNORE: Remove duplicate [GOTO PR 21059] Apr 30, 2023
@rwp0 rwp0 changed the title IGNORE: Remove duplicate [GOTO PR 21059] IGNORE: Remove duplicate [GOTO PR 21059 instead] Apr 30, 2023
@rwp0 rwp0 deleted the rwp0/the-the branch April 30, 2023 00:45
@rwp0 rwp0 changed the title IGNORE: Remove duplicate [GOTO PR 21059 instead] ⚠️ IGNORE: Remove duplicate [▶️ GOTO PR 21059 instead] Apr 30, 2023
@rwp0 rwp0 changed the title ⚠️ IGNORE: Remove duplicate [▶️ GOTO PR 21059 instead] ⚠️ IGNORE: Remove duplicate [▶️ GO TO PR 21059 instead] Apr 30, 2023
@rwp0 rwp0 changed the title ⚠️ IGNORE: Remove duplicate [▶️ GO TO PR 21059 instead] ⚠️ IGNORE: Remove duplicate [GOTO: PR 21059 instead] Apr 30, 2023
@rwp0 rwp0 changed the title ⚠️ IGNORE: Remove duplicate [GOTO: PR 21059 instead] ⚠️ IGNORE: Remove duplicate [GOTO: PR 21059] Apr 30, 2023
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.

None yet

2 participants