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.

Sign up for GitHub

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

Jump to bottom

Project Setup #4

Merged
merged 37 commits into from
Dec 31, 2020
Merged

Project Setup #4

merged 37 commits into from
Dec 31, 2020

Conversation

frinzekt
Copy link
Member

@frinzekt frinzekt commented Dec 8, 2020 •
edited
Loading

The following changes will be made:

  • removal of old code base
  • setup of feathers server
  • setup of nextjs client
  • setup of mkdocs documentation
    • with Continuous deployment on master
  • setup of docker-compose

Related to #2

@frinzekt frinzekt added the enhancement New feature or request label Dec 8, 2020
@mhodki
Copy link

mhodki commented Dec 8, 2020 via email

@frinzekt frinzekt marked this pull request as ready for review December 15, 2020 05:54
MouseAndKeyboard
MouseAndKeyboard reviewed Dec 17, 2020
View reviewed changes
Copy link
Collaborator

@MouseAndKeyboard MouseAndKeyboard left a comment

Choose a reason for hiding this comment

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

Looks good, I will change that secret key which was committed on accident.

@frinzekt
Copy link
Member Author

frinzekt commented Dec 17, 2020 via email

@frinzekt
Copy link
Member Author

@MouseAndKeyboard if you think its good enough, approve it. Also is it possible to change your github name? so I can easily tag

@mhodki
Copy link

mhodki commented Dec 20, 2020 via email

@frinzekt
Copy link
Member Author

frinzekt commented Dec 20, 2020 via email

@mhodki
Copy link

mhodki commented Dec 20, 2020 via email

MouseAndKeyboard
MouseAndKeyboard requested changes Dec 21, 2020
View reviewed changes
Copy link
Collaborator

@MouseAndKeyboard MouseAndKeyboard left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Have secret which was removed in 2d432f4 stored as an environment variable (or similar)

@frinzekt
Copy link
Member Author

Have secret which was removed in 2d432f4 stored as an environment variable (or similar)

can you do that for me and then approve and merge this?

@frinzekt
Copy link
Member Author

@MouseAndKeyboard I need a second review / cleanup on this one. I think this still belongs to project setups

I have made new changes specifically in the client side:

  • file structure and routing (notice this when you go to http://localhost:10004/coordinator/MECH/ . You will see that it will be displaying "MECH" which is the value that can be found in the url. This will be key to all dynamic pages)
  • Login Page layout (custom layout)
  • General Layout for all the other pages (notice this for any files that you see in the folder /pages, you will notice that the admin, coordinator or review pages are mostly empty but they already have an interface, you will see this configuration under _app.js and the folder /components/Layout/*

Note: _app.js is orignally a template file that uses class component. I would like to change this later on to functional component the same way I did the Living Lab website.

If you are confused about any of these, please look at the repository for the Living Lab website, and you will see that we will be using the same pattern, and I have documented the standards there as well. You can also contact me anytime

Also for the secret authentication, I have decided to revert your change, but I do agree that we need to have it in the environment file see production.json for how I did the setup for the production environment. If you don't understand it, mainly the key bit to understand there is that all caps values will be replaced by environment variables (as per the feathers-configuration documentation will say). Another key bit to understand is, default.json is the environment configuration for any environment and is then overwritten by the [environment_name].json eg. production.json, this is the main reason as to why I reverted your commit as I believe that the default configuration is fine, and that we should use the environment variable in development somewhat conservative, but of course, production is the real deal

@frinzekt frinzekt merged commit 5bb83dd into develop Dec 31, 2020
@frinzekt frinzekt deleted the project-setup branch December 31, 2020 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MouseAndKeyboard MouseAndKeyboard MouseAndKeyboard approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frinzekt @mhodki @MouseAndKeyboard