Skip to content
This repository was archived by the owner on Sep 20, 2023. It is now read-only.

Pull Request | Milestone 1.10#58

Closed
BelieveInBunny wants to merge 15 commits intotorrust:v1.1.0from
Piracy-Wiki:main
Closed

Pull Request | Milestone 1.10#58
BelieveInBunny wants to merge 15 commits intotorrust:v1.1.0from
Piracy-Wiki:main

Conversation

@BelieveInBunny
Copy link
Copy Markdown

@BelieveInBunny BelieveInBunny commented May 27, 2022

  • Option for users to only register with an invite link.
  • Enable/Disable User registration or Invite Function

@da2ce7 da2ce7 mentioned this pull request May 27, 2022
Copy link
Copy Markdown
Member

@mickvandijke mickvandijke left a comment

Choose a reason for hiding this comment

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

Hey @BelieveInBunny , thank you for the PR. This has been a much requested feature, so the community will definitely appreciate your work!

But before we can merge this, could you please consider the following changes:

  1. Add backend/DATABASE, backend/DATABASE-shm and backend/DATABASE-wal to .gitignore.
  2. I'm not sure what the purpose of backend/gobang is. Could you explain to me why we need it or else remove it.
  3. Add backend/src/config.toml to .gitignore. The config file should be automatically generated when running the backend.
  4. Delete backend/test.sql and backend/todo. I believe these files are no longer necessary.
  5. Set server.port in frontend/vite.config.js back to 8080.

Thank you.

@BelieveInBunny
Copy link
Copy Markdown
Author

BelieveInBunny commented May 28, 2022

  • 1. Add backend/DATABASE, backend/DATABASE-shm and backend/DATABASE-wal to .gitignore.
  • 2. I'm not sure what the purpose of backend/gobang is. Could you explain to me why we need it or else remove it.
  • 3. Add backend/src/config.toml to .gitignore. The config file should be automatically generated when running the backend.
  • 4. Delete backend/test.sql and backend/todo. I believe these files are no longer necessary.
  • 5. Set server.port in frontend/vite.config.js back to 8080.

yes, working on it now

@BelieveInBunny
Copy link
Copy Markdown
Author

Hello @WarmBeer

  1. I'm not sure what the purpose of backend/gobang is. Could you explain to me why we need it or else remove it.

Regarding this, gobang is database management tool to help edit database. If you think it might be a good idea to keep it. awesome.
If not, we can remove it

@da2ce7
Copy link
Copy Markdown
Contributor

da2ce7 commented May 28, 2022

If you think it might be a good idea to keep it. awesome.

https://github.com/torrust/torrust/blob/90807f92b461ccf50482ae1d4ff78e05efdea8ba/backend/gobang/gobang

It is not good practice to include binaries within the repository. It is better if we remove gobang from this pull request and instead update the Backend's ReadMe File with a link to: https://github.com/TaKO8Ki/gobang

@BelieveInBunny
Copy link
Copy Markdown
Author

It is not good practice to include binaries within the repository. It is better if we remove gobang from this pull request and instead update the Backend's ReadMe File with a link to: https://github.com/TaKO8Ki/gobang

I agree on it, I will note this for future coding

@BelieveInBunny
Copy link
Copy Markdown
Author

Updated

@BelieveInBunny
Copy link
Copy Markdown
Author

I hope everything is good now to merge the request.

@da2ce7
Copy link
Copy Markdown
Contributor

da2ce7 commented May 31, 2022

@BelieveInBunny

Lets first split this into two pull requests:

  1. Enable/Disable New User Registration.
  2. Invite Link Support

This will be easier to review. :) You can start with the more simple "Enable/Disable New User Registration".

@BelieveInBunny
Copy link
Copy Markdown
Author

Thank you, I will close this pull request and create 2 seperate then

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants