we have to distinguish:

• if we set status after clearing database it's always workable
• if we set status after creating/migrating means that schema was created properly and without errors, but something might be broken, and executing operations might fail

the second case is something we don't want to override

If this is just to call expo-secure-store from JS, wonder if we should use their API directly. I guess the downside is that the constants would have to be redefined in JS?

Yea, I had in mind 3 reasons to not use expo-secure-store directly from js:

• redefining constant (I think we can somehow pass this via CommCoreModule to js but it'll increase complexity)
• I wanted to keep logic responsible for setting flags (connected to errors) in one consistent place, in this class instead of dividing this between C++ class/some js functions
• using expo-secure-store is a new pattern in the codebase so I preferred to stick to the approach we already use

But if someone has better arguments against the current solution I am open to changing this

If we call this function and DatabaseStatus is DB_OPERATIONS_FAILURE it is a no-op and the caller is not aware of it. If someone tries to set DatabaseStatus as workable when database is indeed not workable it sounds like a programmer mistake and there should be a way to propagate this information. Maybe we should throw or at least make this function return a boolean indicating that marking database as healthy was not successful?

I would prefer to name this parameter as afterClearingSensitiveData. We never clear SQLite exclusively. However up to you.

Technically, calling expo-secure-store directly from JS doesn't make any difference, so your reasons seem valid to me (maybe besides the last one ;) ).

Returning the value or throwing an error will not change anything since the caller will have to ignore this. I think naming is the problem, how about this one:
Split this into two functions:

• setDatabaseStatusAsWorkable which always sets it as workable (called after clearing sensitive data)
• attemptToSetDatabaseStatusAsWorkable or indicateSchemaCreation or something similar which is called after creating a query executor and will set the database as workable when there was no operations failure?

yea you're right, I will update this before landing

1. Why will the caller have to ignore information that we couldn't set database status because of DB_OPERATIONS_FAILURE?
2. Assuming the answer to 1 convincing then the approach you described is worth implementing in my opinion.
3. If you have an idea on how to improve your diffs, you don't have to wait for any reviewer approving you idea. You can just go ahead, implement it and request review afterwards.

1. Why will the caller have to ignore information that we couldn't set database status because of DB_OPERATIONS_FAILURE?

I edited the code and added the comment, should be understandable right now

2. Assuming the answer to 1 convincing then the approach you described is worth implementing in my opinion.

As we discussed in the office - is convincing

3. If you have an idea on how to improve your diffs, you don't have to wait for any reviewer approving you idea. You can just go ahead, implement it and request review afterwards.

I wasn't sure about this idea, and wanted to verify with someone else