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

[Perl] Generated Languages.pm as part of the build process #176

Merged
merged 14 commits into from
Feb 9, 2025

Conversation

ehuelsmann
Copy link
Contributor

@ehuelsmann ehuelsmann commented Sep 24, 2023

Fixes #32

🤔 What's changed?

The Languages.pm file is now generated upon building the release artifacts or when running the test suite.

⚡️ What's your motivation?

Fixes: #32 (Languages.pm not mergeable)

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)

♻️ Anything particular you want feedback on?

Does this address the concern sufficiently?

📋 Checklist:


This text was originally generated from a template, then edited by hand. You can modify the template here.

@ehuelsmann ehuelsmann force-pushed the generated-perl-languages branch from 18b5c00 to 9fd1084 Compare September 24, 2023 19:22
@ehuelsmann ehuelsmann force-pushed the generated-perl-languages branch 2 times, most recently from c2aa3d4 to 63a22b5 Compare October 13, 2023 22:06
@ehuelsmann
Copy link
Contributor Author

The latest Iteration of this PR generates Languages.pm before testing and release artefact building. It does that by extending the dzil build command. The tests are being run based on what is in the git tree (as they always have). I

f we want to run the tests based on what is in the release tarball, we need to install its content. dzil test and dzil run exists for that purpose: they create an out of tree isolated installation as would be achieved with a vanilla installation. Using dzil this way is not possible, because our testdata needs the tests to run in a relative path to the test data set.

Looking at how Ruby does it, I see it uses bundler . Although I'm not too much into the details, I think the description of what we do now (run from the working tree) is exactly what Ruby does too, since bundle package isn't required for bundle exec and the latter is what the tests use.

Concluding: I think this PR brings the Perl situation in line with what Ruby does.

@ehuelsmann ehuelsmann force-pushed the generated-perl-languages branch from 730a3e1 to 2347a3e Compare February 1, 2025 15:27
@ehuelsmann ehuelsmann changed the title Don't maintain a generated Languages.pm file [Perl] Don't maintain a generated Languages.pm file Feb 1, 2025
@ehuelsmann ehuelsmann changed the title [Perl] Don't maintain a generated Languages.pm file [Perl] Align generation of Languages.pm with Ruby approach Feb 1, 2025
@ehuelsmann
Copy link
Contributor Author

Please let me know what I can do to expedite this PR, or let me know what other direction to take. The PR is created in response to the feedback on the Perl approach with a pre-built Languages.pm file in the repository (which is neither what other languages do, nor desirable, as per the feedback). If a different approach is required, I'll work on that.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Feb 2, 2025

Cheers! I've got next week Thursday planned for core work. So, pending availability, hopefully by then.

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Hey Eric thanks for keeping up with this! It has taken some time. 😅

I've cleaned up the Makefile and pushed some changes so we can run the acceptance tests against the artifact produced by dzil build.

One item I'm not sure how to fix yet is running the "unit test" before the acceptance tests. I think it should work with dzil test but I'm not sure how to get that working in a GitHub action.

Could you help with that?

Copy link
Contributor

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Okay, got something that isn't horrible to work.

Let me know if you see I've committed any sins against Perl. 😉

@ehuelsmann
Copy link
Contributor Author

@mpkorstanje looking very nice! Thanks for ironing out the Makefile stuff and shaping a bit more to your linking. No sins againt Perl paradigms that I can see. I'd say: if it doesn't add new failures, this is an improvement. Let's merge it!

@mpkorstanje mpkorstanje changed the title [Perl] Align generation of Languages.pm with Ruby approach [Perl] Generated Languages.pm as part of the build process Feb 9, 2025
@mpkorstanje mpkorstanje merged commit a14e79a into main Feb 9, 2025
34 of 35 checks passed
@mpkorstanje mpkorstanje deleted the generated-perl-languages branch February 9, 2025 15:49
@ehuelsmann
Copy link
Contributor Author

Thanks for merging! Another action item off my list.

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.

perl: Languages.pm is not mergable
2 participants