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

Add config options for year 2023 in the Makefiles #235

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

noeensarguet
Copy link
Collaborator

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

Comment on lines 48 to 51
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
Copy link
Contributor

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?

Copy link
Collaborator Author

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 ;) )

Copy link
Collaborator

@mdurero mdurero left a 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 Show resolved Hide resolved
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)/)
Copy link
Collaborator

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.

Copy link
Collaborator Author

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

makefiles/variables.mk Outdated Show resolved Hide resolved
@noeensarguet
Copy link
Collaborator Author

noeensarguet commented May 3, 2024

Re: Mathieu's general suggestions, I'm generally ok with them (note that for the source files, it should be ir-calcul/sources$(YEAR)* for it to be factorizable), though I'm not sure I understand what you mean with the last one.
This should probably be done in another separate small PR though, if we keep this one as draft for testing 2023. Happy to do it if you're on board :)

@noeensarguet noeensarguet changed the title [WIP] Add config options for year 2023 (or default) in the Makefiles [WIP] Add config options for year 2023 in the Makefiles May 3, 2024
@noeensarguet noeensarguet marked this pull request as ready for review June 26, 2024 14:17
@mdurero
Copy link
Collaborator

mdurero commented Jun 26, 2024

Re: Mathieu's general suggestions, I'm generally ok with them (note that for the source files, it should be ir-calcul/sources$(YEAR)* for it to be factorizable), though I'm not sure I understand what you mean with the last one. This should probably be done in another separate small PR though, if we keep this one as draft for testing 2023. Happy to do it if you're on board :)

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)/)
Copy link
Collaborator

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.

@noeensarguet noeensarguet changed the title [WIP] Add config options for year 2023 in the Makefiles Add config options for year 2023 in the Makefiles Sep 19, 2024
@noeensarguet noeensarguet merged commit c3e907e into MLanguage:master Sep 19, 2024
1 check passed
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.

3 participants