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

Merge 1.0 branch into master #327

Merged
merged 88 commits into from
Aug 7, 2018
Merged

Merge 1.0 branch into master #327

merged 88 commits into from
Aug 7, 2018

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Jul 31, 2018

The updates here have already been reviewed, mostly in #308. The 1.0 changes should be ready to merge into master now.

Right after merging:

  • Fix broken specification links. I did a search for 3d-tiles/tree/master/ and 3d-tiles/tree/1.0/ and these projects had links:
    • cesium
    • cesium.com
    • cesiumjs.org

@pjcozzi @ggetz is there anything left to add here?

@ggetz
Copy link
Contributor

ggetz commented Aug 1, 2018

List looks good.

Maybe a final copy edit if @slchow has the time, but I don't think it's necessary in order to merge this.

@ggetz
Copy link
Contributor

ggetz commented Aug 1, 2018

Only other thing I can think of is if we want to park current master in a branch because it has the start of some additional tile formats like VectorData, unless we want to carry over those changes here.

@ggetz
Copy link
Contributor

ggetz commented Aug 1, 2018

Disregard my above comment, I see the next branch and #328

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 1, 2018

If not a branch, maybe a tag for the current master so we can reference it later as needed.

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 1, 2018

I pushed a 0.0 tag.

@slchow
Copy link
Contributor

slchow commented Aug 3, 2018

I could copyedit, but it would realistically be the end of next week at least before I could get to it. @lilleyse Do you want me to look it over?

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 3, 2018

I think it's fine to merge without a copyedit.

@ggetz
Copy link
Contributor

ggetz commented Aug 3, 2018

@lilleyse I think we should also merge #329 before merging this.

@ggetz
Copy link
Contributor

ggetz commented Aug 6, 2018

Fixed a few typos and published a new PDF. I think this should be ready to go.

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 6, 2018

I quickly glanced at the pdf - just some small things to address:

The majority of the content in this OGC document is a direct copy of the content contained at https://github.com/AnalyticalGraphicsInc/3d-tiles/tree/1.0 (the 1.0 branch of the 3d-tiles repo). No normative changes have been made to the content. This OGC document does contain content not contained in the 1.0 branch of the 3d-tiles repo.

Would this note need to change? Should it instead refer to the 1.0 tag?

This also reminds me that the current 1.0 tag is now a few commits behind. Could you delete the existing tag and create a new 1.0 tag that points to the latest commit?

In the introduction section, and in other areas, some of the tile formats are written in a plural form, but they should be singular.

Batched 3D Models
Instanced 3D Models
Point Clouds
Batched 3D Model
Instanced 3D Model
Point Cloud

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 6, 2018

Otherwise looks great!

@ggetz
Copy link
Contributor

ggetz commented Aug 7, 2018

Thanks @lilleyse! Made the fixes in the PDF, and now reference the tag instead of the branch.

I don't see any 1.0 tag however, should we create once we merge this PR?

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 7, 2018

Oh I was mixing up the tags...

But yeah, keep the 0.0 tag as-is and add a 1.0 rag right after merging this, and point to the 1.0 tag in the PDF (the link in the PDF should be https://github.com/AnalyticalGraphicsInc/3d-tiles/releases/tag/1.0).

It also looks like some files got added accidentally to the last commit.

@ggetz
Copy link
Contributor

ggetz commented Aug 7, 2018

Sounds good 👍

Whoops, removed the extra files, and updated.

@lilleyse
Copy link
Contributor Author

lilleyse commented Aug 7, 2018

In addition to creating the tag, could you also fix the now-broken spec links mentioned in the PR description?

That looks like everything. Thanks @ggetz!

@lilleyse lilleyse merged commit 3133ce6 into master Aug 7, 2018
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.

4 participants