Skip to content

[Android] Add asin() trigonometric function#95

Closed
mobile-sergey wants to merge 4 commits into
divkit:mainfrom
mobile-sergey:main
Closed

[Android] Add asin() trigonometric function#95
mobile-sergey wants to merge 4 commits into
divkit:mainfrom
mobile-sergey:main

Conversation

@mobile-sergey
Copy link
Copy Markdown

@mobile-sergey mobile-sergey commented Dec 14, 2024

Issue: #79

@mobile-sergey mobile-sergey changed the title Add asin [Android] Add asin() trigonometric function Dec 15, 2024
@lunarstill
Copy link
Copy Markdown
Collaborator

lunarstill commented Dec 26, 2024

Rebase, please. And take a look at this test: https://github.com/divkit/divkit/blob/main/test_data/expression_test_data/functions_trigonometry.json#L251

Sorry for such a late feedback.

@mobile-sergey
Copy link
Copy Markdown
Author

mobile-sergey commented Jan 18, 2025

@lunarstill
I rebase and fix merge conflict - please check that ios is worked correctly.
In test I add platform android to this test.
Sorry for late feedback)

@lunarstill
Copy link
Copy Markdown
Collaborator

Seems the rebase gone wrong, there are unrelated commits.

@mobile-sergey
Copy link
Copy Markdown
Author

But branch has no conflicts with the base branch. May be I rebase to release branch. How to delete unrelated commits?

@grechka62
Copy link
Copy Markdown
Collaborator

But branch has no conflicts with the base branch. May be I rebase to release branch. How to delete unrelated commits?

Hello! You should fetch original repository and rebase your main branch on its main branch. While rebasing there will be conflicts on every commit. You should revert all changes which aren't related with your feature.

You also have to adopt contributor agreement in order to provide contributions. Please, check this instructions: https://github.com/divkit/divkit/blob/main/CONTRIBUTING.md.

@mobile-sergey
Copy link
Copy Markdown
Author

I rebase from main and fix merge conflict. Please check.
I hereby agree to the terms of the CLA available at: [https://yandex.ru/legal/cla/?lang=ru].

]
},
{
"expression": "@{asin(-1.0)}",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There are other tests for asin below. This case seems to be unnecessary because case with parameter -0.5 already exists.
You should add android to platforms in those cases.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Delete this test asin(-1.0) and add "android" to test asin(-0.5)

"value": 0.0
},
"platforms": [
"android"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ios is supported

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Add "ios" to this test

@grechka62
Copy link
Copy Markdown
Collaborator

@mobile-sergey Hi! Please fix issues so we can merge the pull-request.

…pstream

# Conflicts:
#	test_data/expression_test_data/function_signatures_trigonometry.json
#	test_data/expression_test_data/functions_trigonometry.json
@mobile-sergey mobile-sergey reopened this Apr 1, 2025
@mobile-sergey
Copy link
Copy Markdown
Author

Sorry with some troubles. I think that I fix all issues and fix merge-conflicts.
@grechka62 Please check my changes

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants