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

fix(new iot): angular package generation flag for MES >= 10.2.7 #467

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

pedromsilvapt
Copy link

@pedromsilvapt pedromsilvapt commented Jan 20, 2025

The --isAngularPackage flag type was incorrectly marked as string, which was making it so that it was never recognized as true.

Furthermore, there was a bug in the logic of version comparison (missing parentheses were causing, even with the --isAngularPackage flag set to true, on MES versions 10.2.7/8, the generation to happen for ATL packages instead of Angular ones. This part of the code was refactored to make it more readable.

The test cases were updated to cover both the cases where the flag is explicitly passed or not, and for both versions prior and subsequent to 10.2.7. Additionally, a check was added to make sure angular package was indeed being generated (thought the presence of the file angular.json). All tests are passing now:
imagem

Fixes #470.

@m-s- m-s- changed the base branch from main to development January 22, 2025 15:08
jrk94
jrk94 previously approved these changes Jan 23, 2025
@pedromsilvapt pedromsilvapt force-pushed the development-fix-new-iot-angular-packages branch from edcc833 to 98f2fa4 Compare January 24, 2025 15:11
@pedromsilvapt
Copy link
Author

Hi @jrk94, rebased the branch with the conflicts resolved.

I changed the code of the cmf new iot command as well, because the tests were failing due to some recent changes on the LoadDependencies method. I usually don't like changing the code just to fix a test, but in this case, I actually think this solution is actually cleaner than calling that method 😁

@pedromsilvapt
Copy link
Author

Fixes #470

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