search

cosmos / b9-checkers-academy-draft

32 stars 41 forks source link

Update msg_server_test.go #50

Closed francesco-defalco closed 1 year ago

francesco-defalco commented 1 year ago

A simple test on MsgServer

xavierlepretre commented 1 year ago

This file is introduced in the very first commit, the one created with ignite scaffold chain. I am hesitant now as I don't want to squash that in and give the impression that Ignite creates the file like this. This could be something you could submit to Ignite's repo BTW. And adding it to another commit like the 4th "Add checkers rules file" feels a bit artificial 🤔. Maybe I will add to the second commit "Add description files" and add "... and fix linting". Do you have other suggestions for linting on this first commit?

francesco-defalco commented 1 year ago

In general I think there are three ways of move: 1) Like you suggested open an issue on Ignite repo (actually I don't know how/where to open it) 2) Fix it directly in the code in the repository (without say nothing in the online course) since is really a minor issue 3) Introduce in the course that thanks to Ignite CLI are created automatically tests that should be completed and filled by the user

In my opinion the most "formal and correct" way is open an issue on Ignite CLI repo.

xavierlepretre commented 1 year ago

For 1, it looks like opening a PR against this file https://github.com/ignite/cli/blob/main/ignite/templates/module/create/files/msgserver/x/%7B%7BmoduleName%7D%7D/keeper/msg_server_test.go.plush is the way to go. If you find other linting quick wins like that, you can perhaps add them too if you find out that the Ignite team liked your PR.

In the meantime I will add that to the 2nd commit.

xavierlepretre commented 1 year ago

Mmmh, the squash did not preserve your name... https://github.com/cosmos/b9-checkers-academy-draft/commit/8d1c47bff55837a27d1091f0899b025d86831d58