Skip to content

perlxstut add links and update examples #18240

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

Closed
wants to merge 8 commits into from
Closed

perlxstut add links and update examples #18240

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 14, 2020

For #18163.

@ghost ghost mentioned this pull request Oct 14, 2020
@toddr toddr requested review from atoomic and xsawyerx October 14, 2020 15:57
Copy link
Member

@atoomic atoomic left a comment

Choose a reason for hiding this comment

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

LGTM

@karenetheridge
Copy link
Member

Why is perl 5.22.2 needed? Could the commit message be a little more self-explanatory?

@ghost ghost changed the title perlxstut v5.22.2 update perlxstut add links and update examples Oct 14, 2020
@Grinnz
Copy link
Contributor

Grinnz commented Oct 29, 2020

In general it is counter-productive to include unrelated unexplained changes.

@ghost
Copy link
Author

ghost commented Oct 30, 2020

The define PERL_NO_GET_CONTEXT was added to h2xs.PLon Dec 6, 2012.

The define PERL_NO_GET_CONTEXT was added to ParseXS's pods on Feb 16, 2014.

Perl 5.008008 could have been updated to 5.017007 concomitant with either of those commits.

@ghost
Copy link
Author

ghost commented Nov 1, 2020

3 failing checks after bb321ad.

testsuite / Windows msvc142
testsuite / Windows msvc100
testsuite / Windows mingw64

@ghost ghost closed this Nov 1, 2020
@ghost ghost deleted the perlxstut-update branch November 1, 2020 17:14
@xsawyerx
Copy link
Member

xsawyerx commented Nov 1, 2020

@apparluk May I ask why you closed this PR and deleted the branch?

@ghost
Copy link
Author

ghost commented Nov 2, 2020

@xsawyerx

[doc] perlxs rpc (also Closed)

Here are the two files if someone else wants to take up the work.

Gist perlxstut.pod (raw) [deleted Nov 5, 2020]

Contains your code changes (the last all tests passing) and use 5.017007; but not mkmanifest.

Gist perlxs.pod (raw) [deleted Nov 5, 2020]

Run rpctest.pl and verify the correctness of the output before merging.


I have mostly moved on from this document. Please feel free to make whatever changes you feel are necessary to keep it up to date, or expand it as much as you like.

khwilliamson added a commit that referenced this pull request Nov 2, 2020
@khwilliamson
Copy link
Contributor

#18278 has been opened to continue work on xsxtut.pod
#18279 has been opened to continue work on perlxs

@ghost
Copy link
Author

ghost commented Nov 2, 2020

@khwilliamson

cf: #18240 (comment)

I used 5.017007 in the .pod believing PERL_NO_GET_CONTEXT first appeared in that release.

-   use 5.008008;
+   use 5.017007;

However it would be unusual to expose the reader to a development release of perl in the .pod, and maybe using instead the >5.017007 next stable perl version would be more sensible.


cf: #18240 (comment)

The first failed tests on this PR occurred after bb321ad (Nov 1, 2020, 2:31 AM GMT+1).

@ddobbelaere
Copy link

The failing tests are unrelated to the commits in this PR, see #18280

@ghost
Copy link
Author

ghost commented Nov 2, 2020

This PR's msvc fails had the error message citing an unresolved external symbol.

@xsawyerx

This comment has been minimized.

@ghost
Copy link
Author

ghost commented Nov 3, 2020

Keep the append instruction and leave mkmanifest for another PR.

SPECIAL NOTES

Possible expansion could add a subsection on make manifest with mention of ./Build manifest.

@karenetheridge

This comment has been minimized.

This pull request was closed.
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.

6 participants