Skip to content

Conversation

@olmokramer
Copy link
Contributor

Description

Make subclasses of the Directory class to be used instead of the DirTypes enum.
The new classes are: RootDirectory, CourseDirectory, AssignmentDirectory, SubmissionDirectory and RegularDirectory, named after the corresponding values of the enum.
Also moves some functionality specific to those directories to the new classes, e.g. loading submissions is now in the AssignmentDirectory class.

Still WIP as I haven't been able to test it yet, but wanted to get it out there to get some initial feedback on it.

cgfs.py Outdated
assig_dir.insert(AssignmentSettingsFile(cgapi, assig['id']))
assig_dir.insert(
RubricEditorFile(
cgapi, assig['id'], self.rubric_append_only
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work.

cgfs.py Outdated
class AssignmentDirectory(Directory):
def load_submissions(self, latest_only):
try:
submissions = cgapi.get_submissions(self.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to stop using this global variable for new code.

self.stat[key] = value


class Directory(BaseFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

Should FixedDirectory also inherit from this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a class named FixedDirectory?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I meant TempDirectory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It already does...

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm...

@libre-man libre-man force-pushed the master branch 3 times, most recently from d4db8e1 to 29063e5 Compare November 18, 2017 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants