dwyl / learn-travis

:sunglasses: A quick Travis CI (Continuous Integration) Tutorial for Node.js developers
MIT License
1.25k stars 352 forks source link

First time reading, Soshin eyes #23

Closed Fl4m3Ph03n1x closed 6 years ago

Fl4m3Ph03n1x commented 6 years ago

These are my inputs following the first read of this tutorial:

1 - We need an index. An index would allow the reader to know how many sections the tutorial has, and how to quickly access them without spending an eternity scrolling down to try and find the part where he left the tutorial.

2 - Martin Fowler's article is a refreshing read. I suggest that we merely add the bullet points of his paper to the tutorial, and that we redirect to the article for further/detailed reading.

3 - Competitors. Travis sure is great, but if your true objective is to teach CI, then travis is just a tool among many. I suggest, that in the end of the tutorial we make a honorable mention to other CI tools and dwyl resources that people may be interested in (learn-circleci and learn-codeclimate, for example would be good additions that we can link to, since they already exist. We would also be taking advantage of dwyl great repo of knowledge.)

4 - vi is kinda of ... overkill ? nano is usually seen as the easier/most beginner friendly approach. This topic quickly summarizes my argument (see accepted answer): https://askubuntu.com/questions/726669/difference-between-nano-and-vim

5 - Instead of specifying a specific version in your travis.yml, yo can just have "node", and travis will pick the latest version (iirc). Example: https://github.com/Fl4m3Ph03n1x/obj-watcher/blob/master/.travis.yml I defend this would be easier, as beginners would not have to manually maintain their travis environment. A link to travis.yml configuration options could also prove benefitial: https://docs.travis-ci.com/user/customizing-the-build/

6 - Before taking the discussion about specific files, it would be beneficial to offer an overview of the directory structure and its files.

7 - The Pro tip to use travis lint is repeated twice. Once in the encryption section, and once in the "Install Travis-CLI on Ubuntu". Why is that so? Why is this important for the Ubuntu section? I know from reading the previous section where/when I should use it (literally, it is 1 line above).

8 - All the google diagrams are terrible. Oookkkk. Can you specify what you need or what you have in mind? If I know what you are thinking and why you don't like the ones Google has to offer, maybe I or someone else can get started on a nice diagram for your tut :D

This is my feedback. Please let me know which points you approve/deny, and if I can help with any.

nelsonic commented 6 years ago

@Fl4m3Ph03n1x all excellent points. thanks! 👍

iteles commented 6 years ago

@Fl4m3Ph03n1x This is amazing, thanks for adding this issue! They're great points and I'm particularly fond of 1, 2 and 3. Any that you feel happy to PR into this repo, please do!

Fl4m3Ph03n1x commented 6 years ago

I have some spare time now and I would love to leave my mark (albeit small) on this community. I will see what I can do!

Fl4m3Ph03n1x commented 6 years ago

Hey guys! I am here with some updates. I know this guide is one of your (many) babies, so I am threading carefully here.

For my first pull-request I did the following:

For this first request, I focused on point point 1 of my suggestions review.

A few things I am not sure we need:

Ok guys and girls, let me know what you think ! I will move one to the next points once this one gets merged.

Fl4m3Ph03n1x commented 6 years ago

Hello all,

I have pushed my second iteration and I have addressed the following:

I have also rephrased chapters 3-6 in order to have a more coherent sequence. Minor changes to grammar and spelling mistakes.

TODO:

Regarding point 2, I am at a conundrum. I have realized that this tutorial's objective mixes the topics of CI and Travis. The tutorial is called learn-travis, but in reality it is a mix of learn-ci + learn-travis. This mix makes it hard for me to decide on certain decisions. I believe CI is its own topic, and that further changes to this tutorial should remove parts from learn-ci and focus only on travis. In fact, IMHO, before reading this tutorial the user should read before learn-ci, and if learn-ci doesn't exist it should be created to address this need. This is paramount to other learn-X tutorials, which cover tools like Travis but do no cover CI, making the learn-X series confusing with little to no separation of concerns at best.

Regarding point 8, I believe someone created an image for this already? https://github.com/dwyl/learn-travis/issues/17

I personally find it overly complex, but shouldn't it be added somewhere?

For now I feel I can't address points 2 and 8 of my feedback. Let me know what you think!

kfern commented 6 years ago

Hi @Fl4m3Ph03n1x! 👍

This repo was my first resource about CI & Travis. I did learn what is CI and how to with Travis.

If this repo will be about "Learn Travis" we need a "Learn CI" and a tool for learn by doing and Travis is easy and free, so perhaps we could rename this repo to "Learn CI" or "Learn CI with Travis".

Salu2 Fernando

kfern commented 6 years ago

I like toilet photo. I think it's a "Why" a project need CI. But #17 is cool, so we could add it without remove anything

Fl4m3Ph03n1x commented 6 years ago

@kfern CI is independent from travis, as I said in the beginning they are separate things and travis is just a tool among many you can use. I defend the creation of a separate learn-ci tutorial, which would then link to learn-travis, learn-circleci and many others.

As for the photo, where would you add it?

kfern commented 6 years ago

@Fl4m3Ph03n1x I know CI is independent from travis ;-) I'll add Continuous Integration Diagram in https://github.com/dwyl/learn-travis#general-ci-background-reading Do you propose that this tutorial should be only theoretical?

Fl4m3Ph03n1x commented 6 years ago

@kfern I believe the tutorial, as a travis intro is quite practical and mostly to the point.

What I suggest instead, is that we create a separate resouce, called learn-ci, which would be theoretical, and would link to learn-travis, learn-circleci and learn-codeship.

Then we would remove parts explaining ci from learn-travis and make learn-ci a pre-requesite to learn-travis.

This is what I propose. Opinions ?

kfern commented 6 years ago

@Fl4m3Ph03n1x Sorry for the delay in response.

Although the approach you make has sense, I do not like the exclusively theoretical approach.

@nelsonic What do you think about?

Fl4m3Ph03n1x commented 6 years ago

@kfern When you refer to theoretical approach, are you referring to learn-travis or learn-ci? learn-travis would always be a hands on approach, as is now, with an exercise and explaining how to work with it.

Waiting on your feedback guys!

kfern commented 6 years ago

@Fl4m3Ph03n1x I'm referring to learn-ci

Fl4m3Ph03n1x commented 6 years ago

Any news on this? I see the tutorial was changed (logo and everything) but no one else commented on this issue.

nelsonic commented 6 years ago

@Fl4m3Ph03n1x apologies for delay on reviewing your fantastic Pull Request! (notification did not make it to the "top" of my list because PR was not assigned to me ...) PR merged: https://github.com/dwyl/learn-travis/pull/27#pullrequestreview-62285933 Thanks again! Seriously, You are Awesome!!

Fl4m3Ph03n1x commented 6 years ago

@nelsonic glad to know I can help. @kfern and I have divergent opinions on what this guide should be and wether or not we should create learn-ci. What are your 2 cents on this matter?

nelsonic commented 6 years ago

@Fl4m3Ph03n1x learning "CI" in general would certainly be useful as a reference, however we tend to find that people want something specific that "solves" their immediate need (e.g: independently testing their app on Git push).

A more "fundamental" explanation of CI (preferably with a diagram) has been on the "Todo List" for a while ... https://github.com/dwyl/learn-travis/tree/ce3ea29d916f8d6e19dc086d52ccc59bde849fd1#todo

If @kfern (or anyone else) has time to draft something more general I'm sure the community will be keen to read it! 👍

Fl4m3Ph03n1x commented 6 years ago

Ok then, since there seems to be no consensus around the matter and in order to respect what is already done, I vote we leave this tutorial as is and I will close this issue.

If anyone has second thoughts about my suggestions feel free to re-open it. As far as I see, I already contributed with something, so I guess I am happy :P

iteles commented 6 years ago

You've made a fantastic contribution @Fl4m3Ph03n1x , thank you!