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

Add cmake icon #555

Merged
merged 2 commits into from
Dec 19, 2016
Merged

Add cmake icon #555

merged 2 commits into from
Dec 19, 2016

Conversation

Mikayex
Copy link
Contributor

@Mikayex Mikayex commented Dec 19, 2016

Changes proposed:

  • Add
  • Prepare
  • Delete
  • Fix

Things I've done:

  • My pull request fixes an issue, I referenced the issue.

The cmake icon is applied to these files:

  • CMakeLists.txt
  • CMakeCache.txt
  • *.cmake

The icon is applied to these files:
- CMakeLists.txt
- CMakeCache.txt
- *.cmake
Copy link
Member

@jens1o jens1o left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -45,6 +45,7 @@ exports.extensions = {
{ icon: 'coffeescript', extensions: ['coffee'] },
{ icon: 'config', extensions: ['env', 'ini', 'makefile', 'config', 'conf', 'cfg'] },
{ icon: 'config', extensions: ['makefile', '.env.example'], filename: true },
{ icon: 'cmake', extensions: ['CMakeCache.txt', 'CMakeLists.txt', '.cmake'], filename: true, svg: true },
Copy link
Member

Choose a reason for hiding this comment

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

Can you order it alphabetically? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops ! I thought it was but it seems I was tired...
Fixed !

jens1o
jens1o previously requested changes Dec 19, 2016
Copy link
Member

@jens1o jens1o left a comment

Choose a reason for hiding this comment

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

I don't see any change.

@robertohuertasm
Copy link
Member

@jens1o what's the problem?

@Mikayex
Copy link
Contributor Author

Mikayex commented Dec 19, 2016

@jens1o I've made another commit to fix the order : b3fe867

@jens1o jens1o dismissed their stale review December 19, 2016 16:02

Blind eyes.

@jens1o jens1o merged commit 6e15443 into vscode-icons:master Dec 19, 2016
@jens1o
Copy link
Member

jens1o commented Dec 19, 2016

I just looked after the differences in the line deleted and the line added.... There was no difference between them. Until I realized the order itself had been changed ^^

@Mikayex Mikayex deleted the add_cmake branch December 19, 2016 16:04
@JimiC
Copy link
Member

JimiC commented Dec 19, 2016

Guys, the ordering is trivial. The build process assures that the list is sorted.

@jens1o
Copy link
Member

jens1o commented Dec 19, 2016

But it makes the file more readable, isn't it?

@robertohuertasm
Copy link
Member

It's for readability purposes.

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