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

Enable the linuxArm32Hfp Native target #143

Merged
merged 4 commits into from
Jan 15, 2024

Conversation

lppedd
Copy link
Contributor

@lppedd lppedd commented Jan 10, 2024

Other Kotlin libraries, like kotlinx-serialization, had to enable back the linuxArm32Hfp target.
That target is still present, so we should also build for it.

See Kotlin/kotlinx.serialization#2505

@lppedd lppedd marked this pull request as draft January 10, 2024 22:56
@lppedd lppedd marked this pull request as ready for review January 11, 2024 08:27
@lppedd
Copy link
Contributor Author

lppedd commented Jan 11, 2024

@ftomassetti you can review anytime. Had to adjust the readme.

@ftomassetti
Copy link
Member

I would like to get #144 solved before merging other PRs

@lppedd
Copy link
Contributor Author

lppedd commented Jan 12, 2024

@ftomassetti this one can wait, but #146 will block people using certain grammars.
Hopefully the missing person will give permission too.

@ftomassetti
Copy link
Member

We are down to one, so I think that either way we will move forward in a matter of days

| | tvosX64 | |
| | tvosArm64 | |
| | iosArm64 | |

> :warning: The `linuxArm32Hfp` platform is deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want to support it, if it is deprecated?

Copy link
Contributor Author

@lppedd lppedd Jan 15, 2024

Choose a reason for hiding this comment

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

@ftomassetti it is deprecated because it is still under evaluation. It may be supported officially in future versions.

Also, current users of linuxArm32Hfp might benefit from a antlr-kotlin release.
If we will remove it in the future, linuxArm32Hfp consumers will be able to use an older, but working, version of ANTLR.

There is no rumors of this target getting totally removed anytime soon.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, makes sense. I will merge this

@ftomassetti
Copy link
Member

In general it looks good but I would like to understand better why supporting a deprecated target: don't we risk having to remove support in the near future? If so, can we just avoid adding it in the first place?

@ftomassetti ftomassetti merged commit d06c280 into Strumenta:master Jan 15, 2024
6 checks passed
@lppedd lppedd deleted the build/linux-hfp branch January 15, 2024 12:33
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