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

Fix "variable declaration & missing params" #13

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yuvan11
Copy link

@yuvan11 yuvan11 commented Apr 18, 2022

  • Add the missing params in verifyWordWithUyirMei function
  • Fix the incorrect variable declaration in todayLettersMap

@psankar
Copy link
Owner

psankar commented Apr 18, 2022

Hey, Thanks for the MR. May I know what you are trying to solve here ?

@yuvan11
Copy link
Author

yuvan11 commented Apr 18, 2022

Hi @psankar, Just came across your repo through some links from kaniyam foundation & also would like to contribute to tamil OSS projects. Raising the PR is just for improving :)

@psankar
Copy link
Owner

psankar commented Apr 18, 2022

@yuvan11 உங்கள் PRக்கு நன்றி. ஆனால் இது இல்லாத Problem ஐ தீர்க்க முயலுகிறது என்று நினைக்கிறேன் (நான் தவறாகவும் கணித்திருக்கக் கூடும்).

நீங்கள் என்ன பிழை கண்டீர்கள், இந்த PR எப்படி அந்தப் பிழையை நீக்குகிறது என்பதைப் பற்றி நான்கு வரி descriptionல் எழுதுங்களேன். Review செய்ய எளிதாக இருக்கும்.

@yuvan11
Copy link
Author

yuvan11 commented Apr 18, 2022

  • Before, I tried to execute test
    image

  • As you said, it won't be any issue in the existing files, the server works perfectly. But, the test file only fails

  • After , I tried to execute test
    image

This is my understanding, if you find the PR irrelevant, you can close this 😀

@psankar
Copy link
Owner

psankar commented Apr 18, 2022

Thanks. I think the tests needs fixing, but not in this manner. I will look at the MR a few days later and will comment how you can fix this better.

@yuvan11
Copy link
Author

yuvan11 commented Apr 18, 2022

Thanks for your time & clarification. Sure, glad to see how it can be improved better.

@yuvan11 yuvan11 closed this Apr 18, 2022
@psankar
Copy link
Owner

psankar commented Apr 18, 2022

இதனைத் திறந்தே வைத்திருப்போம். என்ன மாற்றம் செய்ய வேண்டும் என்பதை நேரம் கிடைக்கும்போது சொல்லுகிறேன். நன்றி.

@psankar psankar reopened this Apr 18, 2022
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