-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add config options for year 2023 in the Makefiles #235
Conversation
makefiles/variables.mk
Outdated
SOURCE_FILES=$(call source_dir,$(ROOT_DIR)/ir-calcul/M_SVN/$(YEAR)/) | ||
SOURCE_EXT_FILES?=$(call source_dir_ext,$(ROOT_DIR)/m_ext/2022/) | ||
MPP_FUNCTION_BACKEND?=enchainement_primitif | ||
MPP_FUNCTION?=enchainement_primitif_interpreteur |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a default source for any year not in 2019..2022
, but shouldn't this PR add an official block for year 2023?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes definitely, I'll add it (the PR was draft and mainly to get our tests running in the first place ;) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so, looking at the yearly variable selection, I suggest:
-removing the m_spec file everywhere,
-creating a common part
SOURCE_FILES=$(call source_dir,$(ROOT_DIR)/ir-calcul/M_SVN/$(YEAR)/)
SOURCE_EXT_FILES?=$(call source_dir_ext,$(ROOT_DIR)/m_ext/$(YEAR)/)
MPP_FUNCTION_BACKEND?=enchainement_primitif
MPP_FUNCTION?=enchainement_primitif_interpreteur
as these formulas are generic.
- letting in the committed configuration one not needing DGFiP files. All variables can be overloaded at execution time.
makefiles/variables.mk
Outdated
ifeq ($(YEAR), 2022) | ||
ifeq ($(YEAR), 2023) | ||
$(warning WARNING: the source M files and fuzzer tests have not yet been published for year: 2023) | ||
SOURCE_FILES=$(call source_dir,$(ROOT_DIR)/ir-calcul/M_SVN/$(YEAR)/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of maintaining a coherent state for publication, I don’t know if we should already have a 2023 block, at least not a block doing something else than a warning.
2023 case is, for that matters, doable by a generic case, potentially the default one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding was that viewing two different years as two completely different applications implied that having a "default" made no sense for our cases (it would be a dangling cibles.m
that couldn't be linked to any other M files). I'm happy to keep 2023 as a draft PR for testing purposes only as long as there's no officially released M code for 2023
Re: Mathieu's general suggestions, I'm generally ok with them (note that for the source files, it should be |
I’m positively certain the last sentence in my suggestions is utterly unintelligible. Probably because I’m an insufferable conceited person. I think I meant that we should publish a buildable Mlang without any DGFiP file, and that it is actually possible if we continue to allow all variable to be overloaded at the command line. |
ifeq ($(YEAR), 2022) | ||
ifeq ($(YEAR), 2023) | ||
$(warning WARNING: the source M files and fuzzer tests have not yet been published for year: 2023) | ||
SOURCE_FILES?=$(call source_dir,$(ROOT_DIR)/ir-calcul/M_SVN/$(YEAR)/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, no one will ever use Mlang by chance. But given it’s published, I would explain the default directory in the warning, to let people put their sources there.
1524998
to
fdbc6e2
Compare
Should we have a "default" m_ext source file that will be updated following the latest changes?
For now, using the same m_ext file (
cibles.m
) as for 2022. We should make that cleaner, even if it simply consists of copying & pasting in a 2023 directory