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

chore(document/templates): remove all references #192

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

GeorgeMac
Copy link
Contributor

@GeorgeMac GeorgeMac commented Aug 27, 2021

Closes #161
Personally, I think it is time. But, I also understand this is a breaking change.
So I open this to get that conversation rolling again.
I found from our cloud logs the only activity we had was from our post deployment tests.
So I am not worried about cloud (and I think I removed all the code from OSS already 🤔 - if my memory serves).

Update: I stand corrected. It is still there in a read-only form.

@glinton
Copy link
Contributor

glinton commented Aug 27, 2021

If the ui isn't using them, this seems like a fine step in deprecating documents, otherwise just move the documents stuff out of common and into oss and re-generate (make generate-all, though it appears you knew that)

@GeorgeMac
Copy link
Contributor Author

Cheers @glinton
I will go and confirm whether it does or not.

Copy link
Contributor

@glinton glinton left a comment

Choose a reason for hiding this comment

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

✔️ for either way

@williamhbaker
Copy link
Contributor

williamhbaker commented Oct 21, 2021

@GeorgeMac re: making sure the UI doesn't use these paths, I created a branch from this one an re-based it with master so that I could point the UI at it and try out a build. The UI builds fine with this without any typescript errors so I think we can be quite confident that the UI does not use these paths.

Would there be anything else holding up merging this PR? It'd be nice to get this sorted out since OSS already does not have most of these routes and we should deprecate the last remaining read-only route there for consistency.

@GeorgeMac
Copy link
Contributor Author

@wbaker85 that is amazing. Thank you for doing that 🙏 I'm happy to go ahead and merge in that case.

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.

Deprecate/remove the /document/template APIs
3 participants