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

issue/gulp #969

Merged
merged 6 commits into from
Sep 17, 2018
Merged

issue/gulp #969

merged 6 commits into from
Sep 17, 2018

Conversation

tiffy74
Copy link
Contributor

@tiffy74 tiffy74 commented Aug 29, 2018

Updated AngularJS reference document by replacing all references to Grunt with gulp as Grunt was removed from Umbraco build process back in Summer 2017.

Updated AngularJS reference document by replacing all references to Grunt with gulp as Grunt was removed from Umbraco build process back in Summer 2017.
## Overview
Umbraco 7 has a slightly unorthodox project structure, compared to a normal asp.net project. This is by design, a choice from the beginning to embrace a much larger group than "just" the developers who know how to use Visual Studio.

As a result, the Umbraco UI is not a Visual Studio project, but simply a collection of folders and files, following certain conventions, and a small configuration file called `gruntfile` - we will get to the Grunt part in a moment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi tiffy74

Thanks for your update, this is great!

I'm not the best person to review the front end build process, however just skimming through at this point the text mentions 'gruntFile' and 'getting to the Grunt part in a moment', should that now be 'gulpFile' and 'getting to the Gulp part in a moment'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It certainly should!

I believe Grunt has been completely replaced by gulp across all Umbraco so really any documentation that references Grunt should be replaced.

What's useful however is that they're so similar in approach that it's an easy text replacement without having to worry too much about getting the npm commands incorrect because there is virtually no difference.. unless someone tells me otherwise.. but thanks for pointing those out.. not sure how I missed them.

Do I now re-do the pull request or is there an alternative way of making those edits?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @tiffy74 - cool, If you just make the changes in your fork branch, and push, the PR will be magically updated!

@sofietoft sofietoft mentioned this pull request Aug 30, 2018
@tiffy74
Copy link
Contributor Author

tiffy74 commented Sep 4, 2018

Have now swapped out all references to Grunt in favour of gulp :)

dampee
dampee previously requested changes Sep 5, 2018
Copy link
Contributor

@dampee dampee left a comment

Choose a reason for hiding this comment

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

@tiffy74
Copy link
Contributor Author

tiffy74 commented Sep 6, 2018

Yes I can see. Humph. Will fix it asap

Not entirely sure what happened but have now deleted the bad merge text so hoping this will merge fine now.  Always learning x
@jmayntzhusen
Copy link
Contributor

Hey @tiffy74 , the bad merge has indeed been fixed, thanks a lot! 😉

I must admit I don't know enough about the differences in gulp and grunt to know if the console commands are the exact same, if they are - great then we can merge. Will try to test it out with your updated documentation today and get back to you!

Copy link
Contributor

@jmayntzhusen jmayntzhusen left a comment

Choose a reason for hiding this comment

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

Just a couple of small things and then we can merge it 😉

### Grunt
When you have Node.js installed, you need to install Grunt. Grunt is a simple JavaScript task runner, basically like NAnt, MSBuild or any traditional build system [https://gruntjs.com](more about grunt here).
### gulp
When you have Node.js installed, you need to install gulp. gulp is a simple JavaScript task runner, basically like NAnt, MSBuild or any traditional build system [https://gulpjs.com](more about gulp here).
Copy link
Contributor

Choose a reason for hiding this comment

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

[https://gulpjs.com](more about gulp here)
should be
more about gulp here, don't know why that was never caught in the original file 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Would also be great if you capitalized Gulp - if nowhere else then in the title and after a period!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hiya thanks for taking a look. I've fixed the references to the URL link. Thanks for spotting it!

In terms of capitalisation - I decided to keep the lower case as this is how it seems to be referred to in their own documentation. Similarly on the wiki regarding gulp is uses lower case g even within titles whereas the g in Grunt is capitalised. So I concluded that gulp should always be lower case g. However if you disagree then I would be happy to amend it as it is not something I'm going to be too precious about.

In terms of the commands - I did check the syntax from the gulp documentation. It does indeed seem extremely similar. However really appreciate it if you are going to test it within the context of Umbraco. I did run the commands myself before I requested the PR but it always helps to have things verified :)

Once again - thanks. Almost there!

Fixed link reference to gulp.js so that it is formatted correctly.
@jmayntzhusen jmayntzhusen merged commit d4dc7fa into umbraco:master Sep 17, 2018
@jmayntzhusen
Copy link
Contributor

Hey @tiffy74 ,

You're right, all the commands are working and it seems be in order now, thanks for helping out with the Umbraco Documentation #h5yr! 😄

@marcemarc
Copy link
Contributor

Thanks @tiffy74 !!!

hopefully you are able to sleep again at night... and you are not put off contributing again!

@tiffy74
Copy link
Contributor Author

tiffy74 commented Sep 17, 2018

woohoo! Thanks @marcemarc and @jmayntzhusen for your support. And no doesn't put me off at all - makes me wanna do more! :D

@dampee
Copy link
Contributor

dampee commented Sep 17, 2018

Well done @tiffy74 Thanks a lot for your effort! H5YR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants