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

Remove AcConstants.ini #2496

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Remove AcConstants.ini #2496

merged 4 commits into from
Feb 1, 2024

Conversation

thomas-bc
Copy link
Collaborator

@thomas-bc thomas-bc commented Jan 23, 2024

Related Issue(s) #1579 , #1893
Has Unit Tests (y/n)
Documentation Included (y/n) y

Change Description

Fix #1579 and #1893

Remove AcConstants.ini file as it has been replaced by AcConstants.fpp. Updates relevant documentation entries.

May break some of the autocoding capabilities of codegen.py (?), but we are only using the dictionary autocoder which doesn't need it.

Rationale

File is not being used and may cause confusion for users

Testing/Review Recommendations

CI

Other

Associated fprime-tools PR: nasa/fprime-tools#192

@thomas-bc thomas-bc changed the title Issue 1579 Remove AcConstants.ini Jan 23, 2024
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

In general I question if we can do this yet. We need the Ai.xml for making dictionaries, and we need Ac.ini in some form to make the Ai.xml. Thus by deleting this, we theoretically could get corrupted dictionaries.

cmake/autocoder/ai_xml.cmake Outdated Show resolved Hide resolved
Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

Delete the commented out line.

Copy link
Collaborator

@LeStarch LeStarch left a comment

Choose a reason for hiding this comment

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

For those following along at home, the realization is that AcConstants.ini is ignored in XML that was generated from FPP. Thus it is only needed for the 2.x style of writing XML by hand. Since this has been de-scoped, we can merge this!

@LeStarch LeStarch merged commit a3578a0 into nasa:devel Feb 1, 2024
34 checks passed
@thomas-bc thomas-bc deleted the issue-1579 branch June 25, 2024 18:00
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.

Reassess process for adding/editing constants defined in config folder
2 participants