donnell74 / CSC-450-Scheduler

MIT License
1 stars 1 forks source link

Initial Standards merge #33

Closed dhebrink closed 9 years ago

dhebrink commented 9 years ago

We need to merge at least these changes into develop because the guiConstraints.py has been separated into different files and keeps causing a lot of merge conflicts.

prichey commented 9 years ago

Before doing this we should pull develop into standards. We're going to have to deal with merge conflicts anyway - it's probably safer to do on the standards branch rather than develop

CameronHKIng commented 9 years ago

I agree with Preston.

CameronHKIng commented 9 years ago

Also, there are changes here that definitely should not be merged in. Course.py was deleted off of develop, but this merge recreates it. I think that if you're up-to-date with develop, this won't be a problem.

prichey commented 9 years ago

There were also significant changes made in guiConstraints (added / modified functions) that are not reflected here and will break lots of stuff if merged with dev

prichey commented 9 years ago

What was the reason for splitting up gui constraints into separate files? Since they share a lot of the same structure (same imports, some shared functions) it seems to unnecessarily complicate things to have them split up (imo)

renatorangel commented 9 years ago

I separated in different files to fix the warnings of pylint "Too big module". If we don't merge will cause much more trouble than if we merge.

prichey commented 9 years ago

These changes will get merged with develop at some point, it's just a matter of when / how.

Splitting up guiConstraints into two files is fine ultimately, if everything works as it should. The issue is that changes were made to guiConstraints on develop since the files were separated, so the new changes to guiConstraints need to be added to your split up files. What ultimately needs to happen is joining your guiConstraint files back into one, then dealing with any merge conflicts, then splitting back up. I know that's going to be a pain but there's honestly no easy way to do it

dhebrink commented 9 years ago

There is one error I can't pin down in guiClasses.py ~line 725. Some sort of KeyError. I've asked @kylenovak to check it out. Otherwise, I have it running. Its as close to perfect as I can get it.

prichey commented 9 years ago

I just pushed a bug fix. Everything should be all good now

CameronHKIng commented 9 years ago

This pull request includes all the old, sprint 1 standards branch. As it stands, we're merging all the old code in again (see the old course.py).

This branch should have been a new branch off of develop rather than a branch with old code.

dhebrink commented 9 years ago

What files should be removed? I'll make sure they're gone before you merge this into develop. I removed it when I pushed to this branch originally, but I bet when others pulled down and pushed back up, they still had the copies of the old files.

CameronHKIng commented 9 years ago

At the top-level directory: test.csv course.py CSC-Scheduler-Setup.exe

CameronHKIng commented 9 years ago

Other than that, looks good :+1:

dhebrink commented 9 years ago

Once Renato is finished working today, I'll remove those files and push up. Should be good to go after that.

CameronHKIng commented 9 years ago

Sounds good

dhebrink commented 9 years ago

I'm done with what I think needed to be done. Still runs. I'll leave this up to the majority now. @CameronHKIng or Greg I assume can be in charge of actually merging into develop or master.

dhebrink commented 9 years ago

Quick side note, since we have the large guiConstraint.py file, I'll leave it up to someone else if we need to delete the split files (InstructorConstraint, CourseConstraint). I'm not sure what the status of those files are or whether or not they are being referenced.

CameronHKIng commented 9 years ago

@dhebrink Is it just me, or does commit b5e12e0 undo commit 95b7fc6?

dhebrink commented 9 years ago

Its not being "undone", its just being more consistent with changes made in other files. I don't think its anything we need to worry about.