Student developer setup for Yan

[yzhuang52]

This is the student setup for Yan at DSI includes installing serveral tools used for development.

The pull request contains a program written with python that print "Hello". A Dockerfile is provided so that you can run the program inside a docker container simply using command docker build -t image_name on student-developer-setup folder and run docker container run -rm image_name. You should see "Hello" in your console

[yzhuang52]

It's my bad Matthew. At first I'm not sure what does documenting each step means. Max's pr gives me a hint. This time I update a script to run hello.py program and a README file contains each step for setup. Let me know if you have any feedbacks, thanks!

[matthewfeickert]

@yzhuang52 :wave: Because I didn't plan things all the way through :grimacing: when PR #11 got merged it created merge conflicts for everyone else who had a PR open. To get around this (a normal thing in development so no worries) first run

git fetch --all --prune

and then probably the easiest thing to do is attempt to do a merge of the upstream project's main into your feature branch (this one).

git merge upstream/main

You'll get conflicts that you need to resolve. If you aren't familair with this first give it a Google, and if you don't get good results that help let me know and I'll direct you to some. If things go wrong just escape the merge with git merge --abort.

You could also rebase your way to victory, but if you aren't familiar with that yet then I'd hold off on using it.

[yzhuang52]

I'm not sure why I don't have Max code when pulling from main branch. But it seems fine for me to merge my feature branch to main branch. Maybe it will have a merge conflict when mergeing this PR?

[matthewfeickert]

I'm not sure why I don't have Max code when pulling from main branch.

@yzhuang52 Are you pulling from the "upstream" main project?

git fetch --all --prune
git merge upstream/main

(this assumes that you added the upstream as a remote: git remote add upstream git@github.com:AFIDSI/student-developer-setup.git)

Maybe it will have a merge conflict when mergeing this PR?

Yeah, at the bottom of this PR you can see that there are merge conflicts

[matthewfeickert]

@yzhuang52 is this ready for review now? I'm gone to a conference this week with limited internet access. I'll review this later this week.

[yzhuang52]

I believe it's ready for review. I solve the conflicts using the commands you mentioned.

On Wed, Feb 22, 2023 at 2:10 AM Matthew Feickert @.***> wrote:

@yzhuang52 https://github.com/yzhuang52 is this ready for review now? I'm gone to a conference this week with limited internet access. I'll review this later this week.

— Reply to this email directly, view it on GitHub https://github.com/AFIDSI/student-developer-setup/pull/7#issuecomment-1439588373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AX6L6LAXQR7R3DRBMUPIPTDWYXCXVANCNFSM6AAAAAAUFZLI7A . You are receiving this because you were mentioned.Message ID: @.***>

[yzhuang52]

Thanks Matthew,

This is the first time I learned the good PR style. Now I've updated the PR title as well as the PR body.

On Thu, Mar 2, 2023 at 2:56 AM Matthew Feickert @.***> wrote:

@.**** requested changes on this pull request.

This is progress, but in addition to the review comments I've made, as I've mentioned in #7 (review) https://github.com/AFIDSI/student-developer-setup/pull/7#pullrequestreview-1286494542 and #7 (review) https://github.com/AFIDSI/student-developer-setup/pull/7#pullrequestreview-1272113410 you need to revise the PR title and the PR body (main text area).

The PR title should be a short description which gives an overview, though I find it useful to structure it in a similar way to how you would a good commit message https://cbea.ms/git-commit/ — something short that tells you what it will do if you apply it / accept it. Also in general you should add some short text to the PR body (doesn't have to be long) that explains to the maintainer of the project what this PR does, why you're doing it, what Issues it might address.

If you have questions on this, let me know.

In README.md https://github.com/AFIDSI/student-developer-setup/pull/7#discussion_r1122769265 :

+

+3) Pushed to GitHub, created pull request

+

+4) Using Pycharm (Working on Windows System)

+

+5) Created hello.py as a "hello world" program; can be built via the build.sh script.

+

+6-7) Created requirement.txt and add pre-commit as required package

+

+8) Created pre-commit yaml configuration file using its sample official config file

+

+9) Installed docker

+

+10) Read the guide for how to use docker

+

+13) Created a Dockerfile Zhuang folder that builds and runs the "hello world" program

From Issue #5 https://github.com/AFIDSI/student-developer-setup/issues/5 ,

Include this Dockerfile and build instructions or script in your PR.

So either in the README or in a build script you should have steps to build the Docker image and run the image as a container.

In Dockerfile https://github.com/AFIDSI/student-developer-setup/pull/7#discussion_r1122771364 :

+COPY ./Zhuang/requirements.txt ./Zhuang/hello.py ./

+RUN python -m pip install -r requirements.txt

As requirements.txt only contains pre-commit and your Python code has no imports then I don't think you actually need to include the copy or use of the requirements.txt. I appreciate that you might just be trying to get more practice with writing Dockerfiles, but as this should ideally be a runtime environment that contains only the needed dependencies.

In build.sh https://github.com/AFIDSI/student-developer-setup/pull/7#discussion_r1122771714 :

@@ -0,0 +1,3 @@

+#!/bin/sh

+pip install -r requirements.txt

For the same reasons as above, this line can probably be removed.

In README.md https://github.com/AFIDSI/student-developer-setup/pull/7#discussion_r1122766653 :

@@ -1,2 +1,23 @@

-# student-developer-setup

-Bootstrapped instructions for setting up development environments for student developers

+# Yan Zhuang student-developer-setup

+

+1) Created fork on GitHub

+

+2) Created README.md

+

+3) Pushed to GitHub, created pull request

+

+4) Using Pycharm (Working on Windows System)

+

+5) Created hello.py as a "hello world" program; can be built via the build.sh script.

+

+6-7) Created requirement.txt and add pre-commit as required package

This still hasn't been fixed.

— Reply to this email directly, view it on GitHub https://github.com/AFIDSI/student-developer-setup/pull/7#pullrequestreview-1321280559, or unsubscribe https://github.com/notifications/unsubscribe-auth/AX6L6LEIHMTPDZGW32OYXKLW2BOERANCNFSM6AAAAAAUFZLI7A . You are receiving this because you were mentioned.Message ID: @.***>

[yzhuang52]

Dear Matthew,

Sorry for the repeatable requests for review. This time I carefully check your comments as well as the original requirements. Please check my progress via comments and commits.

[yzhuang52]

Sorry for the late response of such simple changes. It's just the final week is coming. I will act faster after passing the final week