Skip to content

Magicseaweed sensor documentation#5598

Merged
arsaboo merged 9 commits intohome-assistant:nextfrom
jcconnell:msw_documentation
Jul 27, 2018
Merged

Magicseaweed sensor documentation#5598
arsaboo merged 9 commits intohome-assistant:nextfrom
jcconnell:msw_documentation

Conversation

@jcconnell
Copy link
Copy Markdown
Contributor

@jcconnell jcconnell commented Jun 25, 2018

Description:

Documentation for the magicseaweed sensor.

Pull request in home-assistant (if applicable): home-assistant/core#15132

Checklist:

  • Branch: Fixes, changes and adjustments should be created against current. New documentation for platforms/components and features should go to next.
  • The documentation follow the [standards][standards].

@jcconnell jcconnell mentioned this pull request Jun 25, 2018
Merged
4 tasks
@frenck frenck added the new-integration This PR adds documentation for a new Home Assistant integration label Jun 25, 2018
frenck
frenck previously requested changes Jun 25, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Hi @jcconnell,

I've reviewed your documentation PR. Could you take a look at the comments? 👍

Comment thread source/_components/sensor.magicseaweed.markdown Outdated
logo: magicseaweed.png
ha_category: Sensor
featured: false
ha_release: "0.72"
Copy link
Copy Markdown
Member

@frenck frenck Jun 25, 2018

Choose a reason for hiding this comment

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

0.72 is already out. You might want to up this to 0.73 ⬆️

Comment thread source/_components/sensor.magicseaweed.markdown Outdated
- max_breaking_swell
```

Configuration variables:
Copy link
Copy Markdown
Member

@frenck frenck Jun 25, 2018

Choose a reason for hiding this comment

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

Please use the configuration tags, for more information please see:
https://home-assistant.io/developers/documentation/create_page/#configuration

Comment thread source/_components/sensor.magicseaweed.markdown Outdated
# Example configuration.yaml entry
sensor:
- platform: magicseaweed
name: Hurricane Spot
Copy link
Copy Markdown
Member

@frenck frenck Jun 25, 2018

Choose a reason for hiding this comment

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

We try to keep all configuration samples minimal. Thus, no optional requirement in the default sample. This helps a user to get started quickly by copy-&-paste the sample without worrying about optional parameters which they most likely not need. If required, insert a full configuration sample later that covers special setups or alike.

Comment thread source/_components/sensor.magicseaweed.markdown
comments: false
sharing: true
footer: true
logo: magicseaweed.png

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

@frenck
Copy link
Copy Markdown
Member

frenck commented Jun 26, 2018

@jcconnell Thanks for the updates, nevertheless, the builds are failing.
Could you take a look? Thx! 👍

@frenck frenck added in-progress This PR/Issue is currently being worked on next This PR goes into the next branch has-parent This PR has a parent PR in another repo labels Jun 28, 2018
@jcconnell
Copy link
Copy Markdown
Contributor Author

jcconnell commented Jul 22, 2018

Hello @frenck, I apologize for the delay. I was asked to write a library to supplement this sensor. Now that it's finished, I'm revisiting this error with the documentation. Jekyll is reporting an error on line 9 column 25 regarding mapping values on sensor.magicseaweed.markdown. I've reviewed the file and searched for similar problems, but I'm still unable to find the issue. Do you see anything I might be missing?

@jcconnell
Copy link
Copy Markdown
Contributor Author

jcconnell commented Jul 22, 2018

Ok, it seems that I have addressed all the problems with my file, sensor.magicseaweed.markdown. It is now complaining about a filename for MQTT logging:

Invalid filename 'components/mqtt/#logging.html'. Deployed filenames cannot contain # or ? characters

@frenck frenck added the needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch label Jul 23, 2018
@frenck
Copy link
Copy Markdown
Member

frenck commented Jul 23, 2018

@jcconnell Rebase this PR onto origin/next, that will probably do the trick.

@jcconnell
Copy link
Copy Markdown
Contributor Author

jcconnell commented Jul 23, 2018

Sorry, I'm not very familiar with this workflow. What would be the process to rebase this onto origin/next? Can it be completed through a web browser?

@MimbaMonkeyHouse
Copy link
Copy Markdown

MimbaMonkeyHouse commented Jul 26, 2018

Did anyone have any luck with this? Tell us if help is needed...

@jcconnell jcconnell force-pushed the msw_documentation branch from ad7fec5 to e191863 Compare July 27, 2018 01:18
MartinHjelmare
MartinHjelmare reviewed Jul 27, 2018
View reviewed changes
Comment thread source/_components/sensor.magicseaweed.markdown Outdated
12AM:
description: Display forecast for 12AM.
spot_id:
description: Surf spot ID to monitor surf forecast of.Details for getting spot id available at [Magicseaweed](https://magicseaweed.com/developer/forecast-api)
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Jul 27, 2018

Choose a reason for hiding this comment

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

There's a missing space after the period before "Details". But maybe rewrite the description somewhat? It reads a bit weird when ending with the preposition in the first sentence.

Comment thread source/_components/sensor.magicseaweed.markdown Outdated
required: true
type: string
monitored_conditions:
description: Conditions to display in the frontend.
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Jul 27, 2018

Choose a reason for hiding this comment

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

This isn't dependent on the frontend per se, but decides what entities to create, ie what data to make available.

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Jul 27, 2018

Can be merged when build passes.

@jcconnell
Copy link
Copy Markdown
Contributor Author

jcconnell commented Jul 27, 2018

Yes! Thanks everyone for the help and patience.

@arsaboo arsaboo dismissed frenck’s stale review July 27, 2018 22:35

Comments addressed and approved by Martin

@arsaboo arsaboo merged commit ecaa94f into home-assistant:next Jul 27, 2018
@ghost ghost removed the in-progress This PR/Issue is currently being worked on label Jul 27, 2018
@frenck frenck removed the needs-rebase The PR has been branched of the wrong base branch or targets an incorrect target branch label Jul 29, 2018
frenck added a commit that referenced this pull request Jul 29, 2018
@frenck
Revert "Magicseaweed sensor documentation (#5598)"
6b924d2 
This reverts commit ecaa94f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@frenck frenck frenck left review comments

Assignees

@jcconnell jcconnell

Labels

has-parent This PR has a parent PR in another repo new-integration This PR adds documentation for a new Home Assistant integration next This PR goes into the next branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jcconnell @frenck @MimbaMonkeyHouse @MartinHjelmare @arsaboo