iScsc / iscsc.fr

The iScsc website, build with passion by wannabe devs 🔥
GNU General Public License v3.0
4 stars 12 forks source link

make the README easier to read #79

Closed amtoine closed 1 year ago

amtoine commented 1 year ago

the README is quite long now :open_mouth: i wanted to have a look, to remind me how to test the website locally but i have some trouble finding my way in it...

i think the README might benefit from one or more of the following

cheers :yum:

ctmbl commented 1 year ago

@amtoine

I totally agree and was feeling the same way, could you propose a PR to address this?

atxr commented 1 year ago

I put it in Good first issue and from my side, I prefer focusing on more technical issues. However, because I wrote most of this README, I can definitely answer questions or provide more information if needed. :call_me_hand:

amtoine commented 1 year ago

@ctmbl @atxr

not really much time really :thinking: if you have time @atxr, especially as you know the document, maybe it would be better to do this asap and i can review :+1:

ctmbl commented 1 year ago

@amtoine @atxr

If no one has much time, we'll delay it a bit, no emergency :+1:

atxr commented 1 year ago

I'll check this then no problem But yes let's delay it!

OlaPom commented 1 year ago

Hi! I came across this thread searching for issus related to documentation. I'd like to get some practice with docs and markdown, and I could help you with the file.

ctmbl commented 1 year ago

Hi @OlaPom ! That could be great actually! I'm sure other maintainers will greatly appreciate your contribution too!

@amtoine already stated pretty all that we want to change about the README.md, if you feel like adding anything else please yourself (or if you think that some parts belong to the wiki :wink:) ! Also, because you're a new contributor to the repo this would be a great occasion for us to know if there is any part of the README.md that lacks clarity or explanation, so tell us if you find any!

One last thing, this is general good practice but I'll say it anyway, please split up your changes in multiple logically separated commits for us to better understand your work :)

Looking forward to see a PR!

amtoine commented 1 year ago

@OlaPom

please go ahead, having outside contributors is really cool :sparkles:

amtoine commented 1 year ago

maybe just a little thing, as it's your first contribution, try to keep it only about style :wink:

if you think the README lacks in clarity, i'd be happy to review another PR :relieved:

OlaPom commented 1 year ago

Thank you for your warm welcome :) I have just sent a PR. I added section numbers and a table of contents. I also corrected some spelling and punctuation errors. You asked about clarity, and I have a few suggestions. First, I would suggest rephrasing a couple of sentences to make them sound more straightforward. For example, “The author assigned to the article will be the name of the authenticated user.” could be put as “Your username will appear as the author of the article.” Second, I think the deployment section could be structured a bit better. It’s a list of steps, but when you scroll through it, it is not clear. You need to read the details to understand what the paragraph is about. My suggestions would be to: change heading titles to start with a verb, use numbered lists, put prerequisites before the task description, and remove quote formatting. Below is an example of what I mean. As it’s not what you asked, I created a separate branch in my forked repository to work on additional changes. For me, it’s always an exercise. Later I can send another PR, and you can see if it is something you’d like to apply.

2.2 Production mode

The production mode allows to deploy the application on the server. To use it, you will need:

a) Set up environment variables(.env file)

Before deploying the application, you need to set the environment variables as for development mode.

cp .env.example .env.production

b) Create SSL certificates

To set up HTTPS, you will need valid SSL certificates.

  1. Comment or delete the whole server section about 443 in the nginx/nginx.conf.template file.
- server {
- listen 443 default_server ssl http2;
- ...
- }

This step is required because the certificates don't exist yet, so they cannot be loaded in the nginx configuration.

amtoine commented 1 year ago

@OlaPom i'll review the PR and then come back to your propositions :wink:

ctmbl commented 1 year ago

@OlaPom

First, I would suggest rephrasing a couple of sentences to make them sound more straightforward. For example, “The author assigned to the article will be the name of the authenticated user.” could be put as “Your username will appear as the author of the article.”

really like this! probably a trace of our poor English :smiling_face_with_tear:

Second, I think the deployment section could be structured a bit better. It’s a list of steps, but when you scroll through it, it is not clear. You need to read the details to understand what the paragraph is about. My suggestions would be to: change heading titles to start with a verb, use numbered lists, put prerequisites before the task description, and remove quote formatting.

seems good but I'd like to see a real diff to feel the improvements! According to me you can open a PR with such changes, but let's wait for @amtoine POV first

amtoine commented 1 year ago

i'm fine with it :+1: