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

Failsafe on missing title for library #880

Merged
merged 2 commits into from May 10, 2023

Conversation

TSurkis
Copy link

@TSurkis TSurkis commented May 4, 2023

Changes

  • Created a new flag for the library builder.
    • Created defaults when the flag is on:
      • name - replaced by "uniqueId" which represents the package name.
      • developers - replaced by an emptyList() if none exist.

Update (07.05.2023)

  • Recoverability is now a default behavior, resulting in the above mentioned scenarios in case of missing data.

## Changes
- Created a new flag for the library builder.
  - Created defaults when the flag is on:
     - name - replaced by `"uniqueId"` which represents the package name.
     - developers - replaced by an `emptyList()` if none exist.
Comment on lines 10 to 12
val firstItem: JSONObject = metaData.getJSONArray("libraries")[0] as JSONObject
firstItem.remove("name")
firstItem.remove("developers")
Copy link
Owner

Choose a reason for hiding this comment

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

I believe this was accidentally left in for testing purposes, please remove

Copy link
Author

Choose a reason for hiding this comment

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

Removed!

@@ -4,9 +4,12 @@ import android.util.Log
import com.mikepenz.aboutlibraries.entity.*
import org.json.JSONObject

actual fun parseData(json: String): Result {
actual fun parseData(json: String, recoverable: Boolean): Result {
Copy link
Owner

Choose a reason for hiding this comment

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

Based on the adjustments, let's not have a recoverable flag for this behaviour, and let it happen automatically.

Especially as this is certainly an non-anticipated behaviour - and practically is not meant to happen (given the meta data is meant to contain that information)

This will also keep the API surface more simple, and does not require a new configuration for the exposed API.

Can you please adjust this

Copy link
Author

Choose a reason for hiding this comment

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

removing :)

* [Library.name] = use [Library.uniqueId]
* [Library.developers] = use [emptyList]
*/
fun recoverableMissingData(recoverable: Boolean): Builder {
Copy link
Owner

Choose a reason for hiding this comment

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

Please drop this function, I don't think it's required to have this behaviour configured.

Copy link
Author

Choose a reason for hiding this comment

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

removing :)

- Added the recoverable functionality as the default behavior.
- Removed accidentally added test code.
@mikepenz mikepenz changed the title Recoverability Feature Failsafe on missing title for library May 9, 2023
@mikepenz mikepenz merged commit 7be18e2 into mikepenz:develop May 10, 2023
2 checks passed
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.

None yet

2 participants