Ability to add component meta #27

GustavoCaso · 2015-10-19T08:15:45Z

This pull request is related with the #24 issue.

This will let user to add meta information about the component, in their #{component}.yml file.
I didn't know the best way of configuring this so I changed a liitle the structure of the yml file, now you need to have stubs that would be an array of components.

There could be some problems for supporting previous components, so I added a extra check on the display of the component, to check it follow the correct format stubs_correct_format? so that way if in the future we change the format the user will know. Also if the user have previous stubs it will show the correct format to him thanks to this method stub_example, it will show an example component yml file.

I will wait for your feedback

houndci-bot · 2015-10-19T08:15:48Z

test/mountain_view/component_test.rb


    assert_instance_of Array, component.styleguide_stubs
    assert_equal expected_stub, component.styleguide_stubs.first
  end

+  def test_component_stubs
+    component = MountainView::Component.new('header')


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

houndci-bot · 2015-10-19T20:58:01Z

test/mountain_view/component_test.rb

+    component_without_stub_file =
+      MountainView::Component.new("social_media_icons")
+    compoenet_with_stubs_but_incorrect_format =
+      MountainView::Component.new('card')


Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

devnacho · 2015-10-22T12:42:21Z

test/generators/component_generator_test.rb

@@ -7,6 +7,7 @@ class ComponentGeneratorTest < Rails::Generators::TestCase
  setup :prepare_destination

  test "Assert all files are properly created" do
+    skip


@GustavoCaso Why do we skip both tests in this file?

@jgnatch They are failing randomly, if you check the travis builds you'll see, also in development they fail randomly, or maybe I'm missing something

We can always re-run them. I think it's preferable to have the tests as false negatives and re-run them than to skip them.

I tried figuring out why it randomly failed but wasn't successful. Any feedback is welcome on that!

houndci-bot · 2015-10-25T16:45:55Z

lib/mountain_view/component.rb

+    end
+
+    def stubs_correct_format?
+      (styleguide_stubs.is_a?(Hash) && styleguide_stubs.key?(:stubs)) || styleguide_stubs.is_a?(Array)


Line is too long. [102/80]

GustavoCaso · 2015-10-26T22:23:51Z

@jgnatch @kitop I have update my code to support Array and Hashes and remove the extra lines for the new hint. But I'm not able to fix the random failing test could give a hand or skip them to merge this pull request and open a new issue to fix this problem ??

Really appreciated, thanks.

kitop · 2015-10-26T22:31:19Z

@GustavoCaso Thanks!!
Re-run the test and now passed—yeah, that should be handled in a different issue/PR.

Will give it a look tomorrow evening :) Thanks again!!!

GustavoCaso · 2015-10-28T10:48:28Z

@kitop how we doing with this one ?

kitop · 2015-10-28T21:13:48Z

@GustavoCaso having some really busy days, sorry for the delay!!
Should be reviewed and merged soon :)

kitop · 2015-10-30T16:21:12Z

@GustavoCaso Sorry for the late response! Everything looks great.
There's only one small thing that I guess it was a bit confusing in the talk.
Regarding the

def self.stub_example
  File.read(File.expand_path("../example.yml", __FILE__))
end

and related code that was changed in: a77f908.

When I mentioned erb, I was thinking of making the yml file and erb, so example.yml.erb so you could interpolate things in there, like the name of the component.
The change introduces some more complexity, and I guess it was because of miscommunication. Sorry about that!

Ideally, going back to the original way it was done, but just changing stub_example to be an instance variable reading from a example.yml file (not even adding the erb stuff now) would simplify the code a bit.
Sorry if I wasn't clear the first time. I can merge and make that change myself if you prefer. Just let me know.

GustavoCaso · 2015-10-30T16:31:20Z

@kitop Don't worry I can do it myself, I really like to help.

So basically what you want is an yml.erb that can interpolate variables ?

kitop · 2015-10-30T16:35:32Z

I'd was thinking of that, going back to how the example was rendered (by just reading a file), but make that file a bit more flexible so we could add component name and make the help a bit more friendly :)

More than happy to hear what you think about it! Sorry again about the miscommunication.

GustavoCaso · 2015-10-31T15:56:24Z

@kitop I tried doing that, loading a file example with extension yml.erb and did not feel natural to me.
Already with current working functionality, there is an example.html.erbthat can by as flexible as you want it to be, passing the component name and all the future functionality added to the component, if the example file needed.

But what I did was added the name of the component to that file and display and example stubs so that the users knows how to proceed if they don't have stubs or the have invalid format.

Still giving support for previous stubs as Array

@kitop I'm open to suggestions, if you don't like this way.

GustavoCaso · 2015-11-02T07:48:39Z

@kitop I have no idea such helper exists, the problem is using srtip_heredoc will place the example_stubs just below the :stubs: array and we don't want that.

A part from this is there anything else you would do differently.

I will wait for your feedback @kitop

kitop · 2015-11-02T14:44:37Z

@GustavoCaso Ah, I see. Thanks for the screenshot :) I guess the :stubs: should be in the string too to be formatted properly, but not a big deal.

Everything looks great! Thanks again for your collaboration. Will probably merge today.

GustavoCaso · 2015-11-02T15:00:55Z

@kitop I moved stubs into the example_stubs now we are using strip_heredoc

When you merge it I will start with the PR #24 for adding extra pages to the style guide

GustavoCaso · 2015-11-05T08:17:41Z

Hey @kitop what is the status with this one ?

Ability to add component meta

kitop · 2015-11-06T18:43:49Z

@GustavoCaso merged. Thanks for your patience!

GustavoCaso · 2015-11-09T09:01:43Z

@kitop Thanks to you. This was my first contribution to open source. I really excited about it.
As mention before I will continue working with the other issue #24

kitop · 2015-11-09T22:56:00Z

Congrats @GustavoCaso!!
All contributions are welcome :)

devnacho · 2015-11-10T11:07:31Z

👏 👏 😀

OddEssay · 2015-12-29T16:32:20Z

Hi guys, do you realise this isn't yet in a tagged release so bundler isn't pulling in the changes?

I got around it by pulling in the master branch directly from github:

gem 'mountain_view', git: 'https://github.com/jgnatch/mountain_view', branch: 'master'

I'm not sure if that's intentional, or just an oversight.

kitop · 2016-01-22T15:32:42Z

@OddEssay totally an oversight! Just released v0.8.0 😄
Thanks for letting us know!

Readme still has to be updated to reflect latest changes, but the code is in there!

GustavoCaso added 4 commits October 18, 2015 23:52

Added few methods for displaying meta information and new yml format

71492e9

Added yaml to prism and a few classes for displaying meta data

ac1b99d

make test pass

c7e33d9

Added new tests for new methods

a410672

houndci-bot reviewed Oct 19, 2015
View reviewed changes

Fix Houndci alerts

86b53bc

houndci-bot reviewed Oct 19, 2015
View reviewed changes

devnacho reviewed Oct 22, 2015
View reviewed changes

This was referenced Oct 22, 2015

Component presenter class #26

Closed

Metadata on the YML file #7

Closed

Fix extra lines for <code> tag and give compatibility for array stubs

ce1d265

houndci-bot reviewed Oct 25, 2015
View reviewed changes

reduce methos length and try new approach for fixing test

1aafb83

Improve help message

a9dc342

Added stubs to example_stubs

84c49b7

kitop added a commit that referenced this pull request Nov 6, 2015

Merge pull request #27 from GustavoCaso/hability-to-add-component-meta

8ef8c67

Ability to add component meta

kitop merged commit 8ef8c67 into devnacho:master Nov 6, 2015

kitop mentioned this pull request Aug 15, 2017

Is it possible to pass a yield in through a yml stub? #59

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to add component meta #27

Ability to add component meta #27

GustavoCaso commented Oct 19, 2015

houndci-bot Oct 19, 2015

houndci-bot Oct 19, 2015

devnacho Oct 22, 2015

GustavoCaso Oct 22, 2015

kitop Oct 22, 2015

houndci-bot Oct 25, 2015

GustavoCaso commented Oct 26, 2015

kitop commented Oct 26, 2015

GustavoCaso commented Oct 28, 2015

kitop commented Oct 28, 2015

kitop commented Oct 30, 2015

GustavoCaso commented Oct 30, 2015

kitop commented Oct 30, 2015

GustavoCaso commented Oct 31, 2015

GustavoCaso commented Nov 2, 2015

kitop commented Nov 2, 2015

GustavoCaso commented Nov 2, 2015

GustavoCaso commented Nov 5, 2015

kitop commented Nov 6, 2015

GustavoCaso commented Nov 9, 2015

kitop commented Nov 9, 2015

devnacho commented Nov 10, 2015

OddEssay commented Dec 29, 2015

kitop commented Jan 22, 2016

Ability to add component meta #27

Ability to add component meta #27

Conversation

GustavoCaso commented Oct 19, 2015

houndci-bot Oct 19, 2015

Choose a reason for hiding this comment

houndci-bot Oct 19, 2015

Choose a reason for hiding this comment

devnacho Oct 22, 2015

Choose a reason for hiding this comment

GustavoCaso Oct 22, 2015

Choose a reason for hiding this comment

kitop Oct 22, 2015

Choose a reason for hiding this comment

houndci-bot Oct 25, 2015

Choose a reason for hiding this comment

GustavoCaso commented Oct 26, 2015

kitop commented Oct 26, 2015

GustavoCaso commented Oct 28, 2015

kitop commented Oct 28, 2015

kitop commented Oct 30, 2015

GustavoCaso commented Oct 30, 2015

kitop commented Oct 30, 2015

GustavoCaso commented Oct 31, 2015

GustavoCaso commented Nov 2, 2015

kitop commented Nov 2, 2015

GustavoCaso commented Nov 2, 2015

GustavoCaso commented Nov 5, 2015

kitop commented Nov 6, 2015

GustavoCaso commented Nov 9, 2015

kitop commented Nov 9, 2015

devnacho commented Nov 10, 2015

OddEssay commented Dec 29, 2015

kitop commented Jan 22, 2016