Skip to content
This repository was archived by the owner on Nov 25, 2019. It is now read-only.

Rhodes documentation map#65

Merged
david-rhodes merged 17 commits intomasterfrom
rhodes-documentation-map
Apr 12, 2017
Merged

Rhodes documentation map#65
david-rhodes merged 17 commits intomasterfrom
rhodes-documentation-map

Conversation

@david-rhodes
Copy link
Copy Markdown
Contributor

Source code documentation pass for Map pieces. See latest published documents here: https://mapbox.github.io/mapbox-sdk-cs/api/Mapbox.Map.html.

I also removed the Update side effects from Map.cs and updated tests.

Copy link
Copy Markdown
Contributor

@isiyu isiyu left a comment

Choose a reason for hiding this comment

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

@david-rhodes 👍
Mostly formatting issues from docFX and referencing and whether we should prefer Console.write over Debug.log

had one note about making "Canonical TileID more explicit"

Comment thread src/Map/CanonicalTileId.cs Outdated
/// </summary>
public struct CanonicalTileId
/// <summary>
/// Canonical tile identifier in a slippy map.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rephrase to something like
Data type to store Web Mercator tile scheme.

Comment thread src/Map/Map.cs
/// data for a geographic bounding box at a certain zoom level.
/// </summary>
/// <typeparam name="T">
/// The tile type, currently <see cref="T:Mapbox.Map.Vector"/> or
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these links weren't building for me locally. doesn't seem like they were building on the to the mapbox.github.io page either?
https://mapbox.github.io/mapbox-sdk-unity/api/mapbox-sdk-cs/Mapbox.Map.Map-1.html

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread src/Map/RasterTile.cs
/// // Handle the error.
/// }
///
/// // Consume the <see cref="Data"/>.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some weird formatting issues here? This is what i'm seeing on the local build:
screen shot 2017-04-11 at 3 29 15 pm

cc @BergWerkGIS

Copy link
Copy Markdown
Contributor

@wilhelmberg wilhelmberg Apr 12, 2017

Choose a reason for hiding this comment

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

@isiyu
We saw xref problems before on Linux/OSX.

Updating docfx solved it.

In root of mapbox-unity-sdk delete

  • file docfx.zip
  • folder docfx

and then run ./scripts/build-docs.sh again.

Does it work now?

Comment thread src/Map/TileCover.cs Outdated
/// Convert a geocoordinate to a TileId:
/// <code>
/// var unwrappedTileId = TileCover.CoordinateToTileId(new Vector2d(40.015, -105.2705), 18);
/// Debug.Log("UnwrappedTileId: " + unwrappedTileId.ToString());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug.Log is Unity specific right?
Should we make this not depend on unity. Console.write should work here in both Unity and Non-unity cases?

Comment thread src/Map/VectorTile.cs
/// // Handle the error.
/// }
///
/// // Consume the <see cref="Data"/>.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

docfx formatting issue:
screen shot 2017-04-11 at 4 11 00 pm

@david-rhodes david-rhodes merged commit 558cb77 into master Apr 12, 2017
@david-rhodes david-rhodes deleted the rhodes-documentation-map branch April 12, 2017 15:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants