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

Fixed macro error #837

Closed
wants to merge 5 commits into from
Closed

Fixed macro error #837

wants to merge 5 commits into from

Conversation

DavidLin1577
Copy link
Contributor

ARM_SAI_ERROR_FRAME_LENGHT -> ARM_SAI_ERROR_FRAME_LENGTH

 ARM_SAI_ERROR_FRAME_LENGHT -> ARM_SAI_ERROR_FRAME_LENGTH
 ARM_SAI_ERROR_FRAME_LENGHT -> ARM_SAI_ERROR_FRAME_LENGTH
 ARM_SAI_ERROR_FRAME_LENGHT -> ARM_SAI_ERROR_FRAME_LENGTH
@JonatanAntoni
Copy link
Member

Hi @DavidLin1577,

thanks for pointing out this typo.

Unfortunately correcting the type is a breaking change to this API.
I am not sure if we can do this that easily. Perhaps we need to keep the misspelled macro for backward compatibility reasons. I need to check with the team.

Cheers,
Jonatan

@ilg-ul
Copy link
Contributor

ilg-ul commented Feb 25, 2020

You can define both, with the incorrect one marked as deprecated.

@DavidLin1577
Copy link
Contributor Author

DavidLin1577 commented Feb 25, 2020

You can define both, with the incorrect one marked as deprecated.

@JonatanAntoni Got it, thanks for your reply. @ilg-ul Thanks for your suggestion and countermeasure. I meet something likes this in my work,"define both" is a good idea and new idea to me,thanks again.

@JonatanAntoni
Copy link
Member

Yep, using @deprecated a viable approach. @DavidLin1577 do you plan to update your PR, accordingly? I'd like to ask you to squash your PR into a single commit, please.

@DavidLin1577
Copy link
Contributor Author

Yep, using @deprecated a viable approach. @DavidLin1577 do you plan to update your PR, accordingly? I'd like to ask you to squash your PR into a single commit, please.

@JonatanAntoni My pleasure, I'm glad to do this better, I will squash my PR into a single commit.

Added (@deprecated) to notice the macro 'ARM_SAI_ERROR_FRAME_LENGHT' had been deprecated, and added macro 'ARM_SAI_ERROR_FRAME_LENGTH' to use.
@JonatanAntoni
Copy link
Member

@DavidLin1577, you need to remove that merge commit and rebase/squash all your commits onto master in order to let me merge your changes properly.

@DavidLin1577
Copy link
Contributor Author

@DavidLin1577, you need to remove that merge commit and rebase/squash all your commits onto master in order to let me merge your changes properly.

@JonatanAntoni thank you for your advise. I got it, but I meet some troubles on this pr when I try serveral times to rebase/squash my commits , I had to fork a new one, and new PR#849. And I will close this pr when #849 finish, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants