gordon-cs / gordon-360-ui

Gordon 360 User Interface
https://360.gordon.edu
16 stars 14 forks source link

Senior project Release #2190

Closed andrew-wzj closed 6 months ago

andrew-wzj commented 8 months ago

Our group has complete the basic components for the housing lottery, and we expect CTS could help us to review our code around spring break, so later it would be secure to merge into develop and master during this semester.

EjPlatzer commented 8 months ago

Hi @andrew-wzj

This Pull Request is to merge the senior-project branch into housing-lottery. I am not sure why you are merging into the housing-lottery branch, can you explain? The housing-lottery branch is very out-of-date with develop (currently 72 commits behind develop), so it doesn't seem like a useful target for a "release".

Also, I see that there are several merge conflicts that need to be resolved. Can you resolve those before I review, so that I am reviewing the most correct version of your changes?

Thanks! Let me know if you have any questions about that.

EjPlatzer commented 8 months ago

@andrew-wzj @ahwnsals1234 @ArabellaJi @JayBae9903, not sure if y'all saw my comment 4 days ago. To summarize, this pull request is to merge into the housing-lottery branch, which is currently 72 commits behind develop. In order for this to be a release PR, I need to see only your changes. Please create a fresh branch based on develop, or else update housing-lottery to be in sync with develop. I will not be able to review this PR until you have done so.

Let me know if you have any questions.

EjPlatzer commented 8 months ago

Hi @andrew-wzj, looks like you updated housing-lottery to be caught up with develop, thanks! I will start reviewing this PR now.

I see that you still have conflicts for package.json and package-lock.json. Those two files are special because they manage the versions of our dependencies. The best way to fix those conflicts is the following:

  1. Follow these instructions to resolve the conflicts via the command line: image
  2. For step four of those instructions ("Fix the conflicts and commit the result"), there is a simple rule to keep in mind: You should never edit package-lock.json yourself. Instead, you should fix the conflicts in package.json, and once that is resolved, you should run npm install, which will tell npm to overwrite package-lock.json with the correct version based on the contents of package.json. Once you have run npm install after fixing the conflicts in package.json, package-lock.json should be conflict-free as well. Then you add both files and commit and push your changes. It seems that the only change you made to package.json in your branch was just to remove caniuse-lite from our list of dependencies (see the history of package.json on the senior-project branch, and the specifics of the commit on December 7th. I'm not sure why you did that, but we need that dependency to make sure our code is compatible with most browsers. So, the way to fix package.json in this case is to overwrite your changes with the changes from develop.

One last note: I am going to close the two PRs you made for merging your branches directly into develop (#2192 and #2193), since we don't want to do that until we're ready to release your code.

andrew-wzj commented 8 months ago

Thanks Evan, I would be working on it rn

andrew-wzj commented 8 months ago

@andrew-wzj @ahwnsals1234 @ArabellaJi @JayBae9903, not sure if y'all saw my comment 4 days ago. To summarize, this pull request is to merge into the housing-lottery branch, which is currently 72 commits behind develop. In order for this to be a release PR, I need to see only your changes. Please create a fresh branch based on develop, or else update housing-lottery to be in sync with develop. I will not be able to review this PR until you have done so.

Let me know if you have any questions. @EjPlatzer Hello Evan, if you're reviewing our code, you might skip the adminView for now since there are too many lines and we haven't split the files into components for the adminView yet, but the studentView should be ready for review

andrew-wzj commented 8 months ago

Hi @andrew-wzj, looks like you updated housing-lottery to be caught up with develop, thanks! I will start reviewing this PR now.

I see that you still have conflicts for package.json and package-lock.json. Those two files are special because they manage the versions of our dependencies. The best way to fix those conflicts is the following:

  1. Follow these instructions to resolve the conflicts via the command line: image
  2. For step four of those instructions ("Fix the conflicts and commit the result"), there is a simple rule to keep in mind: You should never edit package-lock.json yourself. Instead, you should fix the conflicts in package.json, and once that is resolved, you should run npm install, which will tell npm to overwrite package-lock.json with the correct version based on the contents of package.json. Once you have run npm install after fixing the conflicts in package.json, package-lock.json should be conflict-free as well. Then you add both files and commit and push your changes. It seems that the only change you made to package.json in your branch was just to remove caniuse-lite from our list of dependencies (see the history of package.json on the senior-project branch, and the specifics of the commit on December 7th. I'm not sure why you did that, but we need that dependency to make sure our code is compatible with most browsers. So, the way to fix package.json in this case is to overwrite your changes with the changes from develop.

One last note: I am going to close the two PRs you made for merging your branches directly into develop (#2192 and #2193), since we don't want to do that until we're ready to release your code.

When I add the removed dependency back to the package json, and do npm install to resolve the conflicts. It works, but I need to make an another PR for merging the senior-project to housing-lottery since the github doesn't allow me directly commit this change to the current PR

EjPlatzer commented 8 months ago

When I add the removed dependency back to the package json, and do npm install to resolve the conflicts. It works, but I need to make an another PR for merging the senior-project to housing-lottery since the github doesn't allow me directly commit this change to the current PR

@andrew-wzj You should be able to commit your changes to the senior-project branch and commit them. This PR will automatically update to the most recent commit on the senior-project branch.