Skip to content

Conversation

@blondfrogs
Copy link
Contributor

No description provided.

Copy link
Contributor

@cfox cfox left a comment

Choose a reason for hiding this comment

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

Looks good. Nothing concerning except maybe the 30-char check in asset list setup.

REISSUE
};

enum IssueAssetType
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these be moved somewhere into qt codebase since they only seem to apply there?

GetAllOwnedAssets(model->getWallet(), names);
for (auto item : names) {
std::string name = QString::fromStdString(item).split("!").first().toStdString();
if (name.size() != 30)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this check for? I think we could have a 30-char asset name (plus !)..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. So, I am splitting the ! off of these assets names. Then after that if they are < 30 characters. They won't show up because the asset name is already at the limit

void CreateAssetDialog::checkAvailabilityClicked()
{
QString name = ui->nameText->text();
QString name = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

these lines duplicate the block starting at line 160 -- could possibly be consolidated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidated into function

} else {
address = ui->addressText->text();
}
QString name = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consolidated into function

@blondfrogs blondfrogs merged commit 393aa8a into develop2 Aug 22, 2018
@blondfrogs blondfrogs deleted the support_subassets_gui branch September 21, 2018 21:33
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.

3 participants