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

[Documentation:Developer] Updated Installation Instructions #622

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

sahilsuman933
Copy link
Member

Updated the documentation for the MAC installation as well added images for it.

@ziesski ziesski self-requested a review July 23, 2024 14:59
* Steps to edit the env file

1. Go to Submitty install directory and locate the file named "Vagrantfile."
2. Open the "Vagrantfile" in a text editor.
Copy link
Contributor

Choose a reason for hiding this comment

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

Developers should not manually edit the Vagrantfile. Doing so will make it difficult for git commit tracking.

Copy link
Member

Choose a reason for hiding this comment

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

This is indeed true, unless dev puts it in git ignore, it can mess with tracking. On my second thought, I think we should revert this change entirely.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's true that it will make tracking a little bit difficult, but they should be aware of how they can create the env file.

We can do one thing: add one more line about that to put it in gitignore or not to commit it as a note.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gitignore is also a tracked file, so you will run into even more issues by editing it.
We already have instructions on creating the env file, see the last bullet under https://submitty.org/developer/troubleshooting/installation_troubleshooting#forwarded-port-already-in-use
It used to be the first bullet under the heading, but it looks like it got pushed down by later additions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, let me remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fyi the major issue isn't them accidentally committing it, that can be easily caught.
It's that we don't want new devs to have to worry about nasty merge conflicts every time they pull from main.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I get it.

Copy link
Member

@ziesski ziesski left a comment

Choose a reason for hiding this comment

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

Please check the comments.

Copy link
Member

@ziesski ziesski left a comment

Choose a reason for hiding this comment

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

I though the discussion we had above decided to remove the instructions, but it is still there.

@@ -155,7 +155,7 @@ with no explanation, then there are a couple of things that may be going wrong:
```
VM_PORT_SITE=1512 vagrant up
```

Copy link
Member

Choose a reason for hiding this comment

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

this should be reverted

Signed-off-by: sahil suman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Work in Progress
Development

Successfully merging this pull request may close these issues.

4 participants