We have debug flag that enables logging of some additional info for the user. Log texts are "hardcoded" in code and therefore again hardcoded in tests.
Move log message texts to a separate file and then import log messages in code and tests. This way, when you change or improve a log message, you do it in one file and do not touch tests at all
Hello sir , I have cloned the repo and understanding the generator project . I have also watched that "streetlight" tutorial , can you help me out on how to go about things to solve this issue . I am a newbie and I want to contribute .
Thank you sir.
@vishesh13byte hey. the point here is to more all log debug messages like this https://github.com/asyncapi/generator/blob/master/lib/generator.js#L369 to a separate file with constants that would hold the log messages so later we do not duplicate the same log message in tests like https://github.com/asyncapi/generator/blob/master/test/test-project/test-project.test.js#L108.
Ideal world: all log messages are in a single file and in code and tests we just import and use constants and in some cases probably functions (if log message has some variable)
The outcome of this task should be this separate location with log messages and current code where logs are used, and tests, are refactored to use those log messages from new locations. I hope this makes sense.
Tip: @magicmatatjahu did something similar in the past in the react component here https://github.com/asyncapi/asyncapi-react/blob/master/library/src/constants.ts
Hello @derberg. I understand the situation and what one has to do. I can go ahead with making the first draft which will just consist of the plain strings.
However, wanted some more clarity on what to do with messages with variables.
and in some cases probably functions (if log message has some variable)
I understand this but how should I plan to solve scenarios like that of the templateName in the test file which is being used in a significantly different (and dynamic) way in the generator.js file.
I don't want to make the log-messages-constants.js (proposed filename) file too crowded with utility logic.
logsConstants.js should be enough?
in the case of dynamic values inside log messages, you have no other choice than to provide a function where you can pass a dynamic function π€·πΌββοΈ unless there is some better way I'm not aware of. I don't think it is bad, it is just a simple function that returns a simple string and that is it.
more important is to make sure you folks do not duplicate your efforts in the same issue, there are other issues too π
@vishesh13byte is this the issue you are already working on?
@derberg
Yes. I was overcomplicating things when I was thinking of how to do it. π I get it now.
I'll start working on this if @vishesh13byte isn't currently working on it right now. π
logsConstants.jsshould be enough?in the case of dynamic values inside log messages, you have no other choice than to provide a function where you can pass a dynamic function π€·πΌββοΈ unless there is some better way I'm not aware of. I don't think it is bad, it is just a simple function that returns a simple string and that is it.
more important is to make sure you folks do not duplicate your efforts in the same issue, there are other issues too π
@vishesh13byte is this the issue you are already working on?
Yes sir , I am already working on it
@derberg Sir , I have made a pull request . Can you please review my pull request. Thanks for helping me out ,learnt a lot of new things working on it and looking forward to solve more issues and also this issue if anything goes wrong .
Thank you.
@derberg sir , I have done all the necessary changes and the npm run lint is not giving any error and also when I ran command npm run test ,it created a new output file in test folder . I have tried my best in rectifying all the errors ,the latest PR is giving 8 successful checks . Please review my PR sir.
Thank you.
@vishesh13byte that was fast! onto the PR, cheers
@vishesh13byte that was fast! onto the PR, cheers
Thank you @derberg sir for helping me out ,it is a really good start and hopefully I can solve more issues π.
no worries, just keep in mind that you don't have to approach me with sir. In this community we are equal on all levels, even the age, even if I'm older π
I really appreciate this ,thank you for the nice words and the guidanceπ.
:tada: This issue has been resolved in version 1.6.3 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
Most helpful comment
logsConstants.jsshould be enough?in the case of dynamic values inside log messages, you have no other choice than to provide a function where you can pass a dynamic function π€·πΌββοΈ unless there is some better way I'm not aware of. I don't think it is bad, it is just a simple function that returns a simple string and that is it.
more important is to make sure you folks do not duplicate your efforts in the same issue, there are other issues too π
@vishesh13byte is this the issue you are already working on?