GatorIncubator / gatorgrouper

:busts_in_silhouette: Automated Group Formation Tool Enabling Effective Team Work
GNU General Public License v3.0
20 stars 20 forks source link

Merging Develop Branch To Master #154

Closed ibarrezueta closed 5 years ago

ibarrezueta commented 5 years ago

This branch incorporates changes made by multiple teams that fix these issues:

Implementing Django (#142)

A Django app has been added to the root of the project. It's been redesigned a bit to contain modified test files, source files, and models.py file. Along with the Django implementation comes its other files like views.py and urls.py and all of the configuration files. Because implementing Django changed the organization of the repository, changes to the .travis file had to be made as well. In order to simplify the use of this repository, we've installed a Pipfile so that developers could install all of the dependencies with ease. This fixes #94.

Test Files

Because this branch restructures where the tests lie, we had to make changes to the import statements of these files so that pytest can run them without error. They now refer to code in the utils directory instead of the previous directory name.

Source Files

The source files have been grouped together in a directory called utils, that will be used by models.py, the test cases, and views.py. Because the name of their path was changed, the import statements in these files also had to be changed for them to run.

Models.py

This branch holds the current schema for the database in models.py. Django uses this file to create the database, and as such will be the file that needs to be modified if changes to the DB need to be made.

views.py and urls.py

These files will be further used and modified when we import template files that will be displayed by the browser.

Travis

We've reconfigured Travis so that it points to the correct location of the tests files.

Pipfile and more (#150)

Because we switched from having a file with dependency names to using pipenv, we've added a pipfile so that developers can use pipenv install --dev to create a virtual environment with all of the project dependencies.

@huangs1

100 Delete Travis CI Integration in README file

@yeej2

101 Move the original badge (which was also changed from coveralls to codecov) with some additional new badges (build status and language) to the top of the README file and edit details of Activation Coveralls in README file

@ilikerustoo

Removes Google Spreadsheet Integration (#140)

spreadsheet.py was removed, along with relevant implementation, to simplify command-line application and web design. Fixes #97.

quigley-c commented 5 years ago

@aubreypc @sutterj addressed this concern in the other PR #142

@aubreypc You can't rename these directories without renaming the app or the project. When you run startproject and startapp, you have to provide a name. These are then the names that it gives to the directories.

...

It sounds like it won't be possible to rename these directories. Is this incorrect?

ibarrezueta commented 5 years ago

Thanks for the PR! A couple points of feedback:

* The name of the `gator_grouper` package should be changed, as it is confusing to have both `gator_grouper/` and `gatorgrouper/`. Especially since there are files with the same name in both dirs, like `urls.py` -- the distinction should be obvious just from looking at the structure of the project.

* Does the database file need to be committed to the repo, or will Django create one automatically on startup if it doesn't exist?

@aubreypc The database does need to be there cause it holds superuser info and other information

sutterj commented 5 years ago

@aubreypc @quigley-c It is correct that you cannot rename these. The outer directory is the name of the project and the inner directory is the name of the app. You could start over in order to give them new names and then copy in the files that you changed (making sure that you change the app name where appropriate), but I don't really think that it is necessary. The distinction between these directories and the structure of the directories is all an inherent part of Django.

aubreypc commented 5 years ago

@quigley-c It is possible to change the initial startproject and startapp names, at least according to this blog post. If that doesn't work, we could run startproject again and then copy in any files which were modified from that base template.

ibarrezueta commented 5 years ago

@aubreypc What would you recommend the name changes be?

sutterj commented 5 years ago

@aubreypc @barrezuetai I would seriously recommend against changing them in the existing files. There are likely to be a slew of issues so if you are going to change them, run the startproject and startapp commands again. There are a lot of files that are created in the background that are going to have the old names on them that have the potential not to get changed. Django is pretty finicky/particular in this area.

aubreypc commented 5 years ago

@barrezuetai Personally I would suggest renaming gator_grouper to config to distinguish the Django project configuration from the gatorgrouper app -- does anyone else have suggestions?

sutterj commented 5 years ago

@aubreypc @barrezuetai I think that the name of the app and the project should somehow name the tool. I would call them something like gatorgrouper_project and gatorgrouper_app. I would recommend not using config as Django uses .config and AppConfig. You could end up with something like config.config.

Michionlion commented 5 years ago

@aubreypc @sutterj @barrezuetai I would suggest looking into the idea of Django apps -- @ilikerustoo and I looked into that but it turned out to be too complicated for this restructure. This could enable renaming gator_grouper to gatorgrouper and gatorgrouper to applications or something similar.

There's no reason not to be able to rename the folders, as they are all just specified in the generated python files.

ilikerustoo commented 5 years ago

@aubreypc @sutterj @barrezuetai I would suggest looking into the idea of Django apps -- @ilikerustoo and I looked into that but it turned out to be too complicated for this restructure. This could enable renaming gator_grouper to gatorgrouper and gatorgrouper to applications or something similar.

There's no reason not to be able to rename the folders, as they are all just specified in the generated python files.

@Michionlion I will look into it. Although I am not familiar with django, it should not be too much of a problem to rename some things. We need to make this project easy to understand so that the people who work on Gatorgrouper after us do not run into too many problems. @aubreypc I agree with the name change and discussed it with @barrezuetai and @Michionlion a few days ago. This was part of my reasoning for getting rid of the gatorgrouper_django root folder.

Lancasterwu commented 5 years ago

There is an empty test folder in the root of the repository which does not make any sense, I think we should not have that. However, I am getting [Errno 21] Is a directory: '/opt/python/3.7.1/lib/python3.7/distutils/tests' (parse-error) when I am trying to delete it.

Lancasterwu commented 5 years ago

The students.csv file seems necessary for now. The testing team (@GatorEducator/team-lancaster and @GatorEducator/team-spencer ) would not be able to run the restructured program without an input file. We should put it back, and remove it after the json feature is successfully implemented.

ilikerustoo commented 5 years ago

@lancasterwu please refer to my PR #156 before making more commits. The changes you mentioned have already been fixed.

Lancasterwu commented 5 years ago

Thanks @ilikerustoo, just want to make sure . Is bugfix/hide-secret-key the most recent branch with everything updated? Or the correct and working branch can only be the develop branch after both bugfix/hide-secret-key and bugfix/fixing-bugs-in-develop merged in?