Skip to content

Desmakers 4279 rename the metafile vendor from infineon to infineon #331

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

Conversation

LinjingZhang
Copy link
Collaborator

By creating this pull request you agree to the terms in CONTRIBUTING.md.
https://github.com/Infineon/.github/blob/master/CONTRIBUTING.md
--- DO NOT DELETE ANYTHING ABOVE THIS LINE ---

CONTRIBUTING.md also tells you what to expect in the PR process.

Description
rename the metafile vendor from infineon to infineon

Related Issue
Desmakers 4279

Context
Also need to adapt the documentation.
Will update the other PR.

@LinjingZhang LinjingZhang marked this pull request as ready for review March 18, 2025 19:39
@jaenrig-ifx
Copy link
Member

jaenrig-ifx commented Mar 18, 2025

Mmm, we are adding this as a 3.x change?
I think this can break existing installation with the low case "I". Have this been tested?
Any issue with bringing this change in 4.x?

Also, as this is only for xmc .... ideally we could name the package_infineon_index.json -> package_xmc_index.json. But that will definitively would be a "major" compatibility break. But this will require the index url to be updated, otherwise the later versions will not be automatically available to the users.
And for that we should add some notification in the build or such... So that they are aware and can upgrade to the new version.

@ederjc
Copy link
Member

ederjc commented Mar 18, 2025

Yes, this might have broader implications than we initially thought. Also regarding all build checks involving this core.
Is there a reason to merge this to 3.x except the compatibility problems with the PSOC core?

Also, please add required changes to /tests/Makefile.test in this PR, to make it complete.

@LinjingZhang
Copy link
Collaborator Author

The only reason for merging to 3.x is the compatibility problem with PSOC.
I will discuss with Juan and decide which branch to merge into.

Yes, this might have broader implications than we initially thought. Also regarding all build checks involving this core. Is there a reason to merge this to 3.x except the compatibility problems with the PSOC core?

Also, please add required changes to /tests/Makefile.test in this PR, to make it complete.

Regarding Makefile.test, this will be updated anyways and may have different fqbn name depending on how the core is developed. This is just a reference document.

@LinjingZhang LinjingZhang force-pushed the DESMAKERS-4279-rename-the-metafile-vendor-from-infineon-to-infineon branch from eaef614 to aa1cebe Compare March 20, 2025 18:04
@LinjingZhang LinjingZhang changed the base branch from master to 4.0.0-pre-release March 20, 2025 18:05
@LinjingZhang LinjingZhang force-pushed the DESMAKERS-4279-rename-the-metafile-vendor-from-infineon-to-infineon branch from aa1cebe to 41f034d Compare March 20, 2025 18:18
@LinjingZhang
Copy link
Collaborator Author

Mmm, we are adding this as a 3.x change? I think this can break existing installation with the low case "I". Have this been tested? Any issue with bringing this change in 4.x?

Also, as this is only for xmc .... ideally we could name the package_infineon_index.json -> package_xmc_index.json. But that will definitively would be a "major" compatibility break. But this will require the index url to be updated, otherwise the later versions will not be automatically available to the users. And for that we should add some notification in the build or such... So that they are aware and can upgrade to the new version.

now merge to 4.0.0. Another reason to merge to 3.x first is, the compilation tests fails anyways in 4.0.0. I tested with 3.x so the changes should be fine be merged.
Other naming problem you mentioned are under : https://jirard.intra.infineon.com/browse/DESMAKERS-4159 ( you already created 😄 )

@jaenrig-ifx
Copy link
Member

Mmm, we are adding this as a 3.x change? I think this can break existing installation with the low case "I". Have this been tested? Any issue with bringing this change in 4.x?
Also, as this is only for xmc .... ideally we could name the package_infineon_index.json -> package_xmc_index.json. But that will definitively would be a "major" compatibility break. But this will require the index url to be updated, otherwise the later versions will not be automatically available to the users. And for that we should add some notification in the build or such... So that they are aware and can upgrade to the new version.

now merge to 4.0.0. Another reason to merge to 3.x first is, the compilation tests fails anyways in 4.0.0. I tested with 3.x so the changes should be fine be merged. Other naming problem you mentioned are under : https://jirard.intra.infineon.com/browse/DESMAKERS-4159 ( you already created 😄 )

You can also disable/comment for the time being all the tests which aren´t yet ready. And you can focus on the error of the modules/lib that you are working on.

@ederjc ederjc self-requested a review March 21, 2025 12:10
Copy link
Member

@ederjc ederjc left a comment

Choose a reason for hiding this comment

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

Sorry I missed this:
Please rework the commit messages to fit the format and correct spelling.
Our commit checks also seem to be not running properly for this PR?
https://github.com/Infineon/XMC-for-Arduino/actions/runs/13989397821

@LinjingZhang
Copy link
Collaborator Author

LinjingZhang commented Mar 21, 2025

Sorry I missed this: Please rework the commit messages to fit the format and correct spelling. Our commit checks also seem to be not running properly for this PR? https://github.com/Infineon/XMC-for-Arduino/actions/runs/13989397821

It fails because we check the commit msg log until master-HEAD. A PR squash merged before we introduced commit msg checking.
I will rather keep that squashed commit and just skip that one.

@LinjingZhang LinjingZhang requested a review from ederjc March 21, 2025 13:03
@jaenrig-ifx
Copy link
Member

what about changing the boards.txt names also snake_case?

@LinjingZhang
Copy link
Collaborator Author

what about changing the boards.txt names also snake_case?

https://jirard.intra.infineon.com/browse/DESMAKERS-4280
https://jirard.intra.infineon.com/browse/DESMAKERS-4282

@jaenrig-ifx
Copy link
Member

what about changing the boards.txt names also snake_case?

https://jirard.intra.infineon.com/browse/DESMAKERS-4280 https://jirard.intra.infineon.com/browse/DESMAKERS-428

It makes sense to bring all these changes at one in one PR, with all the compatibility breaking changes.

@@ -68,19 +68,11 @@ def diagnose_subject_line(subject_line, subject_line_format, err):
def verify(sha, err):
verbose("verify", sha)
err.prefix = "commit " + sha + ": "

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain the changes on this file?
At some point, this will be moved to the reusable workflow for all cores. So it is good to align on the changes of these checks scripts.

Copy link
Collaborator Author

@LinjingZhang LinjingZhang Mar 24, 2025

Choose a reason for hiding this comment

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

Before commit message with ignore prefix still checks the legitimacy of the email, and it can't accept noreply anonymous emails, so if the PR is squash merged or user is using anonymous email, the check will fail.
Old logic: check all email formats, check commit message format except for prefixes
New logic: check email and commit message format except for prefixes

Copy link
Member

Choose a reason for hiding this comment

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

okay, so this disables commit signing?

Copy link
Collaborator Author

@LinjingZhang LinjingZhang Mar 24, 2025

Choose a reason for hiding this comment

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

Disabled only for commit messages starting with ignore_prefixes = [“squash!”, “fixup!”, “amend!”, “WIP"].
Otherwise commit signing will still be checked.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, we will harmonize this when we make it reusable.

@LinjingZhang
Copy link
Collaborator Author

what about changing the boards.txt names also snake_case?

https://jirard.intra.infineon.com/browse/DESMAKERS-4280 https://jirard.intra.infineon.com/browse/DESMAKERS-428

It makes sense to bring all these changes at one in one PR, with all the compatibility breaking changes.

OK I understand. Then we should align better during scrum planning next time.

@LinjingZhang LinjingZhang force-pushed the DESMAKERS-4279-rename-the-metafile-vendor-from-infineon-to-infineon branch from f8bafc7 to c4faaa2 Compare March 25, 2025 10:43
@LinjingZhang LinjingZhang changed the base branch from 4.0.0-pre-release to 4.0.0-metafiles-naming-conventions March 25, 2025 10:46
Copy link
Member

@jaenrig-ifx jaenrig-ifx left a comment

Choose a reason for hiding this comment

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

👍

@LinjingZhang
Copy link
Collaborator Author

move the commit msg check to: #332

@LinjingZhang LinjingZhang merged commit 98ab007 into 4.0.0-metafiles-naming-conventions Mar 25, 2025
5 of 11 checks passed
@LinjingZhang LinjingZhang deleted the DESMAKERS-4279-rename-the-metafile-vendor-from-infineon-to-infineon branch March 25, 2025 10:54
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