Skip to content

Conversation

@HPWebdeveloper
Copy link
Owner

@HPWebdeveloper HPWebdeveloper commented Jan 13, 2024

@HPWebdeveloper
Copy link
Owner Author

HPWebdeveloper commented Jan 13, 2024

FYI, @3m1n3nc3 @SSEsmaeeli


@SSEsmaeeli you can send a new PR to the branch (types) to conclude this PR (Types#16)?

This time please:

  • Check for any other missing types including those in your recent PR
  • Don't remove any PHPDocs like it was removed in that PR, They should be kept for additional information, and they are also automatically sanitized by the GitHub flow.

@HPWebdeveloper HPWebdeveloper merged commit d661d59 into main Jan 14, 2024
@HPWebdeveloper HPWebdeveloper deleted the types branch January 14, 2024 11:25
@3m1n3nc3
Copy link
Contributor

Removing the optional parameters for the log reference generator introduces another small problem, for instance, I use the Valorin\Random\Random::string() to generate log reference, and this allows me to pass parameters that determines the length of the string, if I need uppercase, lowercase, numbers or symbols in the generated string. Now I am limited to only providing the lenght of the string.

@SSEsmaeeli
Copy link
Contributor

SSEsmaeeli commented Jan 14, 2024

There are a couple of considerations here:

  • This package is not a generator package, so a simple generator class would be enough to handle the token generation. If someone needs more details, he or she can implement his or her own thoughts and be able to achieve what you said.

  • The previous code won't give more chances to the person who is using the package. The only parameter is length, which is considered inside the code as $refGen[2].

     `$refGen = config('pay-pocket.log_reference_generator', [
          Str::class, 'random', [config('pay-pocket.log_reference_length', 12)],
      ]);
      
      $reference = config('pay-pocket.reference_string_prefix', '');
      
      $reference .= isset($refGen[0], $refGen[1])
          ? $refGen[0]::{$refGen[1]}(...$refGen[2] ?? [])
          : Str::random(config('pay-pocket.log_reference_length', 12));`
    

@HPWebdeveloper
Copy link
Owner Author

@3m1n3nc3

Thank you for sharing your feedback and concerns and the attention to detail in highlighting the impact of removing optional parameters for the log reference generator. It is truly valuable, as will help make improvements to this open source project.

I will carefully consider your suggestion to ensure that we maintain the flexibility and functionality that the users have come to rely on.

Please rest assured that your feedback is important to me, and I am committed to consider or delivering a solution that addresses these concerns. If you have any further thoughts or ideas, please don't hesitate to share here. I am always eager to hear and work together to enhance the project.

@HPWebdeveloper HPWebdeveloper mentioned this pull request Jun 26, 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.

4 participants