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

Adding plugin migration page. #3249

Merged
merged 1 commit into from Nov 19, 2019

Conversation

sfshaza2
Copy link
Contributor

@sfshaza2 sfshaza2 commented Nov 15, 2019

For some reason, I am unable to stage this branch. The tool/serve.sh script doesn't work.

  • Adding a new plugin migration page. (I started with the page from the wiki, and added new front matter.)
  • While I was in that directory, editing all the markdown files to use our link convention and also to break on phrases. Just clean up stuff.

Please review, or delegate to the correct person. Thx!

@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Nov 15, 2019
libraries to ensure backwards compatibility. The `android.support`
libraries are deprecated, and replaced with
[AndroidX]({{site.android-dev}}/jetpack/androidx/).
Android code often uses the [`android.support`][]
Copy link
Member

Choose a reason for hiding this comment

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

There are a few instances of replacing links with [foo][]. Why is this preferable? It seems like it's easier to end up with broken links.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is our convention. The text is far more readable without the inline link. And Travis tells us of broken links.

Copy link
Member

Choose a reason for hiding this comment

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

Is Travis a human or a bot? It would nice to have the validation automated since it's almost impossible to review visually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Travis is a CI tool. We used to use it on flutter/flutter, but now it's Cirrus. Very much a bot. :) . We also run a weekly linkcheck over the site.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, phew. I didn't want a real Travis to be hurt about me not knowing if they were a bot or not. I think bot Travis will get over it.

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

LGTM. It would be helpful if you updated the PR message to include the other changes you've made as well.

@sfshaza2 sfshaza2 merged commit 3f85e28 into staging-add-to-app-do-not-delete-until-2020 Nov 19, 2019
@sfshaza2 sfshaza2 deleted the plugin branch November 19, 2019 00:28
@@ -4,18 +4,17 @@ description: How to fix AndroidX incompatibilities that have been detected by th
---

{{site.alert.note}}
You might be directed to this page if the framework detects a problem in your
Copy link
Member

Choose a reason for hiding this comment

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

I thought I left some comments here yesterday but I can't find it anymore. It might not have gone through.

I unfortunately have to revert the non-add-to-app changes in this add-to-app branch. Keeping 2 branches is difficult but feasible for purely additive changes. But for changes on existing files like this, it's hard to merge since the base text already changed on master (specifically for this file, it already moved on master). I tried to manually downport some of the non-add-to-app changes back to master but since the base text was so different, I think it's going to be easier to just re-create these changes directly on an up-to-date master.

TL;DR the new plugin-api-migration.md and sidenav.yml will be mergeable to master but the rest will be difficult.

xster pushed a commit that referenced this pull request Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Contributor has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants