Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Folder restructuring - #30 and #31 #87

Closed
wants to merge 5 commits into from
Closed

Conversation

lpatmo
Copy link
Member

@lpatmo lpatmo commented Feb 22, 2020

Addresses #30 and #31.

(I also took a stab at #50, but renaming cbv3_django_prototype to app everyone broke a lot of things -- so am holding off that for now. Will share screenshot of error messages in #50.)

Implementation

[x] Move all project files under the cbv3_django_prototype directory to the repository root
[x] Consider reorganizing the project, so that apps are created in the project root (rather than in the nested directory)
[x] Move all existing apps (namely, resources, users, and userauth) to the project root

Thanks @brylie for these suggestions, and @billglover and @BethanyG for reminding me I signed up to tackle them!

The app looks good (is still running heh) with these changes, so I shouldn't have broken anything. Please let me know if you noticed that I missed something.

@BethanyG
Copy link
Member

I've probably pulled this incorrectly, but Django won't start for me, and all tests are failing under this structure. Thoughts?

@lpatmo
Copy link
Member Author

lpatmo commented Feb 22, 2020

@BethanyG Can you share a screenshot of the error messages you're getting?

When I checked out this branch again after being in master everything was fine, but then I did docker-compose down anew and docker-compose up --build and saw this error:

image

:(

@lpatmo lpatmo added the hold label Feb 22, 2020
@lpatmo
Copy link
Member Author

lpatmo commented Feb 22, 2020

Earlier I'd thought I was only getting that after changing the top-most folder name to app, but maybe the issue started earlier...

@BethanyG
Copy link
Member

Apologies. Just getting back to this. Yup. That's the same error I am getting:

image

@brylie
Copy link
Member

brylie commented Feb 23, 2020

Make sure the users app is configured correctly in settings.py and that Django is finding the settings files after the shuffle :-)

@lpatmo
Copy link
Member Author

lpatmo commented Feb 23, 2020

@brylie Thanks! Do you have a good understanding of what needs to happen in (what I assume) config/settings/base.py? Am still having trouble debugging this issue: django.core.exceptions.ImproperlyConfigured: Cannot import 'cbv3_django_prototype.users'. Check that 'users.apps.UsersConfig.name' is correct.

@brylie
Copy link
Member

brylie commented Feb 24, 2020

Do a project-wide search for cbv3_django_prototype to see if there are any files where it is still being used, such as for imports.

I'll see if I can find a more detailed guide to renaming the core folder.

@brylie
Copy link
Member

brylie commented Feb 24, 2020

To reduce confusion, the backend/cbv3_django_prototype/cbv3_django_prototype folder can be renamed to something like backend/cbv3_django_prototype/core. Renaming it to app would be confusing to developers since in Django "Applications" means something different than the configuration directory we are renaming.

Double-check the following files:

wsgi.py

sys.path.append(os.path.join(app_path, "cbv3_django_prototype"))

settings.py

APPS_DIR = ROOT_DIR.path("cbv3_django_prototype")

MIGRATION_MODULES = {"sites": "cbv3_django_prototype.contrib.sites.migrations"}

manage.py

sys.path.append(os.path.join(current_path, "cbv3_django_prototype"))

@brylie
Copy link
Member

brylie commented Feb 24, 2020

@lpatmo I opened a pull request for your branch.

#89

Would you please test those changes?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* Rename project folder

* Change cbv3_django_prototype references to project

* Rename 'core' directory

* Change references to users app

* Change references to 'core' directory

* Change project name to "Codebuddies"

* Change path to /opt/codebuddies

* Fix project path
@billglover
Copy link
Contributor

Is this still on hold or can I remove the hold tag and open this one up for merging in?

@BethanyG
Copy link
Member

I'd like to hold this for a bit more, since we've already begun work on both a projects endpoint and the centralization of tagging code. Suspect that merging this will require a "blow away and re-do" for those who have old trees, so want to make sure pending code changes are all checked in and re-worked beforehand.

@lpatmo
Copy link
Member Author

lpatmo commented Feb 27, 2020

Suspect that merging this will require a "blow away and re-do" for those who have old trees

Yes, am afraid of that happening, since I had to do blow away the repo to see it working!

I'm wondering if we can merge this in after tags and before additional work on the projects endpoint, though, since I mainly proposed projects so @bkbuilt could have something to start tackling. Do you have an issue with a good stopping point there, @bkbuilt?

@brylie
Copy link
Member

brylie commented Feb 27, 2020

At the worst, it should only be necessary to delete and then check out the master branch locally. In general, since Git was tracking the structural changes they should be applied when people pull the master branch. They may also want to merge changes from master into any working branches.

@brylie
Copy link
Member

brylie commented Feb 27, 2020

Another thing is that the new tags app can simply be placed in the project folder while in development. That way the changes are already in the correct location.

@BethanyG
Copy link
Member

BethanyG commented Mar 6, 2020

@lpatmo - do we want to hold work and bite the bullet now that tags is merged in? I was going to start work on some testing related issues, but we should get this piece done first, I think.

@lpatmo
Copy link
Member Author

lpatmo commented Mar 7, 2020

@BethanyG I will work on this asap (tonight or tomorrow); thanks for reminding me! And also for merging in #96 🎉

@lpatmo
Copy link
Member Author

lpatmo commented Mar 7, 2020

There were a couple of conflicts here, and in the process of solving it I found it easier to create a new PR here: #99

The new PR should include both Brylie and Bethany's changes.

If someone would be so kind as to check out the issues-30-31-file-reorg-part-2 branch from #99 and double check that it runs and approve it, I would be grateful! Thank you!

@lpatmo lpatmo closed this Mar 7, 2020
@lpatmo lpatmo reopened this Mar 7, 2020
@lpatmo
Copy link
Member Author

lpatmo commented Mar 7, 2020

Update: I've merged in PR#99. Thanks again @brylie for your commits! :)

@lpatmo lpatmo closed this Mar 7, 2020
@billglover billglover deleted the issues-30-31-file-reorg branch March 7, 2020 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants