Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Updated known errors #112

Merged
merged 8 commits into from
Oct 17, 2022
Merged

Updated known errors #112

merged 8 commits into from
Oct 17, 2022

Conversation

ayushtiwari110
Copy link
Contributor

I tested the app on windows and mac. I was not able to arrange Linux so didn't test it on Linux.

As discussed, I have updated the errors which I saw on windows and which were known to me.
The instructions are already nice so I didn't feel they need to be simplified or so...
Kindly have a look, and if everything seems good, accept this PR and mark it as Hactoberfest accepted.

@ayushtiwari110
Copy link
Contributor Author

hello @mturoci , kindly have a look and accept this PR

Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ayushtiwari110! A few comments from me.

README.md Outdated



**3. Python not found:** This error is probably if Windows is not able to recognise the path of Python. The path of Python can be updated in the pyvenv.cfg file, located in [App Folder]/venv
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If python is not found, then venv cannot be created, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes , this error is shown atmost beginning and nothing works in that case, as venv not works.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as venv not works

Then The path of Python can be updated in the pyvenv.cfg file, located in [App Folder]/venv is an invalid statement, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually , In the app folder there is a pre loaded folder venv which comes when we clone the repo.
Now in this venv folder ,there is pyvenv.cfg file. So this file and the venv folder comes with the app. So if python is not found, it is because in this preloaded file, the address is wrong
And the Make file takes the path of python from here i guess.
So i believe this is valid statement. please do update me if anything wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have committed and specified this thing in the file to avoid confusion. :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the app folder there is a pre loaded folder venv which comes when we clone the repo

I have looked through the repo, but could not find the venv as you say. Can you point out where you see it?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh thanks for highlighting that. Yes it is not there. I guess that error was specific to my pc due to difference in directories of python installation and wave-apps.
So I guess it will be good to remove this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The missing python installation is a valid error and should be documented. You correctly included it, the only problem was your problem resolution. Please put it back with the proper way for fixing it - installing python, putting it to the PATH etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As requested, I have added the python error. Kindly check

@ayushtiwari110
Copy link
Contributor Author

I have commited the changes suggested by you. kindly check, and if everything's fine mark as hacktoberfest-accepted

@ayushtiwari110
Copy link
Contributor Author

@mturoci thanks for highlighting those points. I have updated the changes as requested. Kindly see.

@ayushtiwari110
Copy link
Contributor Author

@mturoci kindly check

ayushtiwari110 and others added 3 commits October 17, 2022 12:35
Co-authored-by: mturoci <64769322+mturoci@users.noreply.github.com>
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ayushtiwari110, good job!

@mturoci mturoci merged commit d135df2 into h2oai:main Oct 17, 2022
@ayushtiwari110
Copy link
Contributor Author

Thanks! Nice time working on this , please do add hacktoberfest accepted tag also

@mturoci
Copy link
Collaborator

mturoci commented Oct 17, 2022

Merging is enough to count towards your Hacktoberfest submissions, no worries :)

@ayushtiwari110
Copy link
Contributor Author

Okay, thanks! Learned new things while working on this :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants