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

Adds support for restricted wallets and description for logs and improve overall type annotations. #9

Closed
wants to merge 17 commits into from

Conversation

3m1n3nc3
Copy link
Contributor

@3m1n3nc3 3m1n3nc3 commented Jan 3, 2024

See Issue #8

@3m1n3nc3 3m1n3nc3 changed the title Adds support for restricted wallets and description for logs Adds support for restricted wallets and description for logs and improve overall type annotations. Jan 3, 2024
@HPWebdeveloper
Copy link
Owner

This pull request attempts to address two distinct areas: enhancing type annotations and introducing a new feature. These concerns are sufficiently separate to warrant individual pull requests. Therefore, it is required you create two distinct pull requests.

Regarding the feature-related pull request, it's imperative to adhere to two crucial requirements. Firstly, provide a comprehensive explanation of the feature's functionality and its applicable use cases. Secondly, ensure that tests are written to cover all conceivable new scenarios thoroughly.

Your contribution is greatly valued, and these steps will enhance the clarity and effectiveness of the project.

Thank you for contributing.

@3m1n3nc3
Copy link
Contributor Author

3m1n3nc3 commented Jan 4, 2024

Following your recommendations, I opened a new pull request that contains tests and a few major changes to this pull request, #10

@HPWebdeveloper
Copy link
Owner

@3m1n3nc3

There are some PHPDocs you have previously add/corrected/modifed in this closed PR

https://github.com/HPWebdeveloper/laravel-pay-pocket/pull/9/files

You can now bring them as a separate PR.

@3m1n3nc3
Copy link
Contributor Author

3m1n3nc3 commented Jan 11, 2024

@3m1n3nc3

There are some PHPDocs you have previously add/corrected/modifed in this closed PR

https://github.com/HPWebdeveloper/laravel-pay-pocket/pull/9/files

You can now bring them as a separate PR.

@HPWebdeveloper Can we start with the restricted/allowed wallets feature?

@HPWebdeveloper
Copy link
Owner

@3m1n3nc3

have you checked my questions mentioned here?

Please explain and let me know the use case you have experienced.

#8

Thanks

@HPWebdeveloper
Copy link
Owner

HPWebdeveloper commented Jan 12, 2024

@3m1n3nc3

can you bring all types correction you previously did and PHPDoc blocks like these as a separate PR?

these are examples, I saw you have corrected also some type hints. please bring them as well in a single PR.

interface WalletOperations
{
    /**
     * Get User's Wallet Balance
     *
     * @return int|float
     */
    public function getWalletBalanceAttribute(): int|float;

    /**
     * Get the balance of a specific wallet type.
     *
     *
     * @param string $walletType
     *
     * @return float|int
     */
    public function getWalletBalanceByType(string $walletType): float|int;

    /**
     *  Check if User's wallet balance is more than given value
     *
     * @param int|float $value
     *
     * @return bool
     */
    public function hasSufficientBalance(int|float $value): bool;
    
    ```

@3m1n3nc3
Copy link
Contributor Author

@3m1n3nc3

can you bring all types correction you previously did and PHPDoc blocks like these as a separate PR?

these are examples, I saw you have corrected also some type hints. please bring them as well in a single PR.

interface WalletOperations
{
    /**
     * Get User's Wallet Balance
     *
     * @return int|float
     */
    public function getWalletBalanceAttribute(): int|float;

    /**
     * Get the balance of a specific wallet type.
     *
     *
     * @param string $walletType
     *
     * @return float|int
     */
    public function getWalletBalanceByType(string $walletType): float|int;

    /**
     *  Check if User's wallet balance is more than given value
     *
     * @param int|float $value
     *
     * @return bool
     */
    public function hasSufficientBalance(int|float $value): bool;
    
    ```

Should the PR contain any code reference to #12 or should be based off the current state of the main branch?

@HPWebdeveloper
Copy link
Owner

The new PR (let's say PR-types) should be pure and independent of any other features.

Make a new branch from the current Main branch.

Please don't add any new code. Just bring those types and phpdocs that you have already coded in this closed PR and not add more.

Regarding the new feature (allowed wallets) we will discuss in another open issues/PR.

@3m1n3nc3
Copy link
Contributor Author

The new PR (let's say PR-types) should be pure and independent of any other features.

Make a new branch from the current Main branch.

Please don't add any new code. Just bring those types and phpdocs that you have already coded in this closed PR and not add more.

Regarding the new feature (allowed wallets) we will discuss in another open issues/PR.

@HPWebdeveloper see #13

@HPWebdeveloper HPWebdeveloper mentioned this pull request Jan 13, 2024
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.

2 participants