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

Cleanup - remove unused parameter with magic numbers #22871

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from

Conversation

happy-barney
Copy link

NewOp's first parameter is unused since 2007, polluting codebase with magic numbers.

PR is adding new macro NewOp_v542 without this unused parameter.

Macro name is also POC of adding Perl version when macro will be added to maintain reasonable
backward compatibility by version string in symbol name.

@iabyn
Copy link
Contributor

iabyn commented Dec 23, 2024 via email

@happy-barney
Copy link
Author

happy-barney commented Dec 23, 2024

I can't see what practical gain you get by including the version number in the new macro. It just seems to make it worse.

Symbol name is describes behaviour. With new version behavior changed (3 parameters instead of 4).

It's same like with humans. Look for example at French kings, there were for example:

  • Louis XIV
  • Louis XVI

They have version in names as well, so you can distinguish one born in 17th century and another in 18th.

Same is for this pattern - you can introduce new symbol, new behaviour and still provide old one so there is no need to modify every old code simultaneously.

@iabyn
Copy link
Contributor

iabyn commented Dec 23, 2024 via email

@happy-barney
Copy link
Author

But the version number tells you nothing about the behaviour.

Well, difference is that I'm using release numbers and not version per symbol.
There it tells you one important thing - when it was introduced. Rest is matter of documentation.

I'm not using version to provide different behaviour with of same symbol. It will still be NewOp behaviour
with two implementations:

  • as implemented before v5.42
  • as implemented in v5.42

It may not tell much about actual behaviour, but it tells which version of behaviour and it ensures that it will be preserved.

Following this pattern ensures 100% forward compatibility (eg: no need to recompile XS with each release).

It will even (with some tweaks) allow to propagate newer grammar into older versions - this is root assumption behind idea of exact use VERSION - https://github.com/happy-barney/perl-wish-list/blob/master/exact-use-version/spec/spec.md

Branislav Zahradník added 2 commits December 24, 2024 13:13
NewOp's first argument has been unused since 2007 but remains required,
polluting the codebase with magic numbers.

A new macro is provided that eliminates these magic numbers while maintaining
backward/forward compatibility through Perl version identification in its name.
First parameter of NewOpSz is unused since 2007.
@happy-barney happy-barney force-pushed the hpb/unused-magic-numbers branch from 0032ed0 to 63ab1d4 Compare December 24, 2024 12:13
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.

2 participants