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 scaling issue #177

Merged
merged 1 commit into from Dec 6, 2021
Merged

Conversation

zaptoopa
Copy link
Contributor

@zaptoopa zaptoopa commented Dec 6, 2021

Hi @niqdev I just tested the new version 2.3.1. Unfortunately, I found that I implemented something wrong in the last PR; the scaling didn't work properly. Here I have a fix that I have tested extensively. Sorry for the hassel.

@niqdev
Copy link
Owner

niqdev commented Dec 6, 2021

no worries, and thanks for taking the time to review it!

@niqdev niqdev merged commit 115e333 into niqdev:master Dec 6, 2021
@niqdev
Copy link
Owner

niqdev commented Dec 7, 2021

the release of 2.3.2 is failing, there is a mem issue apparently

 * What went wrong:
Metaspace

@zaptoopa @hannesa2 could you please pull the latest version and checkout what configurations are you using locally? we might need to override them - I don't have the sdk installed or the project running locally atm. Thank you!

This might have been caused by the latest gradle upgrade btw

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

the release of 2.3.2 is failing, there is a mem issue apparently

 * What went wrong:
Metaspace

I would simply push the re-run action button.
image

I can't because of missing rights

@niqdev
Copy link
Owner

niqdev commented Dec 7, 2021

i did that 3 times already, checkout the attempts. there might be something else

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

This might have been caused by the latest gradle upgrade btw

I don't think so, this pull request was successful. But hold on, pull requests are just do ./gradlew assembleDebug with separated ./gradlew check and release do ./gradlew clean build at once.
The error comes from here :app:lintAnalyzeDebug. There is no need for it in release

I would recomment to do on release only
./gradlew assembleRelease
instead off
./gradlew clean build
btw this clean is obsolete

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

an other solution could be
image

@zaptoopa
Copy link
Contributor Author

zaptoopa commented Dec 7, 2021

could you please pull the latest version and checkout what configurations are you using locally?

Gradle Plugin Version = 7.0.3
Gradle Version = 7.3.1
JAVA = 11.0.10

The only thing I set is the JAVA version (v11); the other values are my basic settings. The build run without issues on AVD with API 29 and 30.

I see only warnings and that PreferenceManager in IpCamDefaultActivity.kt and IpCamSnapshotActivity.kt is deprecated. It could be solved using implementation 'androidx.preference:preference-ktx:1.1.1'

@zaptoopa
Copy link
Contributor Author

zaptoopa commented Dec 7, 2021

ohh, ok, after cleaning and rebuilding i see the warnings

clean_n_rebuild

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

@zaptoopa you should run ./gradlew :app:lintAnalyzeDebug

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

Local the error prone task
image

@zaptoopa
Copy link
Contributor Author

zaptoopa commented Dec 7, 2021

I have some warnings like memcpy that i can trace back to the commit #161 , but in the end the build is:

BUILD SUCCESSFUL in 17s
53 actionable tasks: 53 executed

Should i paste the output here?

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

A complete ./gradlew clean build like in CI job works local

image

For me this is the evidence to have a problem on CI machines only

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

I guess this is the solution on CI #178

Only this is needed 8ad9fbe

@hannesa2
Copy link
Contributor

hannesa2 commented Dec 7, 2021

The other commit 0e24f78 only slows down CI

@zaptoopa
Copy link
Contributor Author

zaptoopa commented Dec 7, 2021

For me this is the evidence to have a problem on CI machines only

yes, i agree, especially since it is built locally

@niqdev
Copy link
Owner

niqdev commented Dec 7, 2021

Thanks for the quick feedback! yes i think it's just a problem on the CI machine

@zaptoopa
Copy link
Contributor Author

zaptoopa commented Dec 7, 2021

thx @hannesa2, @niqdev nice, it seems to be ok now. will the release be bumped to version 2.3.2?

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.

None yet

3 participants