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.

Sign up for GitHub

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

Jump to bottom

Add first version of app-params database and checker #23

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Add first version of app-params database and checker #23

merged 1 commit into from
Apr 21, 2023

Conversation

xchapron-ledger
Copy link
Contributor

@xchapron-ledger xchapron-ledger commented Apr 3, 2023 •
edited
Loading

@xchapron-ledger xchapron-ledger force-pushed the xch/app-params-check branch 4 times, most recently from 8ce229c to 8751380 Compare April 3, 2023 13:41
@xchapron-ledger xchapron-ledger force-pushed the xch/app-params-check branch 5 times, most recently from 33347f8 to 2b428bc Compare April 4, 2023 10:19
lpascal-ledger
lpascal-ledger reviewed Apr 12, 2023
View reviewed changes
Copy link
Contributor

@lpascal-ledger lpascal-ledger left a comment

Choose a reason for hiding this comment

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

The rationale and its implementation seem good and practical. I don't have any remarks other than implementation details.

scripts/gen_database.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/makefile_app_params_check.py Outdated Show resolved Hide resolved
scripts/gen_database.py Outdated Show resolved Hide resolved
scripts/gen_database.py Outdated Show resolved Hide resolved
@xchapron-ledger xchapron-ledger force-pushed the xch/app-params-check branch 3 times, most recently from bf16d97 to 090ec2d Compare April 13, 2023 13:18
xchapron-ledger
xchapron-ledger commented Apr 18, 2023
View reviewed changes
Copy link
Contributor Author

@xchapron-ledger xchapron-ledger left a comment

Choose a reason for hiding this comment

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

Add check on appName unicity

xchapron-ledger
xchapron-ledger commented Apr 20, 2023
View reviewed changes
.github/workflows/_check_app_params.yml Outdated
with:
repository: LedgerHQ/ledger-app-database
path: ./ledger-app-database
ref: xch/app-params-check
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to remove before merge

@xchapron-ledger xchapron-ledger mentioned this pull request Apr 20, 2023
Merged
@xchapron-ledger xchapron-ledger force-pushed the xch/app-params-check branch 2 times, most recently from 25c3bb7 to e78802d Compare April 21, 2023 09:45
@xchapron-ledger xchapron-ledger merged commit c8c5855 into master Apr 21, 2023
@xchapron-ledger xchapron-ledger deleted the xch/app-params-check branch April 21, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpascal-ledger lpascal-ledger lpascal-ledger left review comments

@yhql yhql yhql left review comments

@tdejoigny-ledger tdejoigny-ledger tdejoigny-ledger left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@xchapron-ledger @yhql @lpascal-ledger @tdejoigny-ledger