Repository navigation
Conversation
|
I reviewed this PR, there are some logic flaws. Before: if team.type == .free
{
if requiredAppIDs > availableAppIDs
{
if let expirationDate = sortedExpirationDates.first
{
throw OperationError.maximumAppIDLimitReached(appName: application.name, requiredAppIDs: requiredAppIDs, availableAppIDs: availableAppIDs, expirationDate: expirationDate)
}
else
{
throw ALTAppleAPIError(.maximumAppIDLimitReached)
}
}
}After: if team.type == .free
{
if requiredAppIDs > availableAppIDs
{
if let expirationDate = sortedExpirationDates.first
{
throw OperationError.maximumAppIDLimitReached(appName: application.name, requiredAppIDs: requiredAppIDs, availableAppIDs: availableAppIDs, expirationDate: expirationDate)
}
}
}removing the check isn't going to help just like that, needs more explanation on why the fix attempts to remove the error in else part |
|
@mahee96 if you look at the previous commit linked in the pull request it removed the check. when it was added back in it it caused a regression and broke it for previous dev accounts. that check breaks it for previous dev accounts because the team type is always free for previous dev accounts. |
|
@duronald I do understand that removing it fixes, but the check is supposed to guard it since it is already in scope of free account, the actual bug is since u said urs is a paid dev account it isn't supposed to come into this if-block at all... that is why it kinda needs further investigation into where actually it is picking the account wrongly as free instead of hacking the check here by removing it. if you are interested u can look into it, otherwise myself or some other sidestore team member like @nythepegasus will take a look and do a clean fix because we don't want to break something else by fixing this. |
|
@duronald I am converting this PR to a draft for now, if u have more inputs let us know. |
|
sounds good, i don’t have time to look at it more deeply but it’s understandable. I’ll fork it into a different repo for now. |
|
yes if u need fix immediately u can build from source actually and install via Xcode. |
9a5b9c4 to
3961688
Compare
|
@duronald, we are sorry, but there has been significant refactors due to recent new developments. This might have been fixed, or still out there, can u rebase your branch and stage the PR once again for review? |
|
Unfortunately I currently do not have time to look into this but am still using SideStore, I’m going to keep my fork since there are a few others using it as well but when I have time I will take a look. @mahee96 |
|
@mahee96 I did a quick test and can confirm the issue is still present in the latest nightly. I will keep my fork for now since there are at least 5 others using it. |
|
@duronald can u confirm if this still an issue in latest nightly |
Changes
See issue
#1156
and previous issue
#410
related commits:
551c004 (where issue was originally fixed)
3682b65 (where issue reappeared)