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

Add a v1 => v2 migration guide #1098

Merged
merged 12 commits into from Apr 6, 2020
Merged

Conversation

TomOnTime
Copy link
Contributor

@TomOnTime TomOnTime commented Mar 29, 2020

What type of PR is this?

(REQUIRED)

  • bug
  • cleanup
  • documentation
  • feature

What this PR does / why we need it:

(REQUIRED)

A guide to migrating from v1 to v2 makes it easier for people to adopt the newer version; helps eliminate the long tail of people using old versions.

Which issue(s) this PR fixes:

(REQUIRED)

Fixes #921

Special notes for your reviewer:

n/a

Testing

I viewed it in GitHub's preview mechanism.

Release Notes

(REQUIRED)

n/a

Initial version of a v1-to-v2 migration guide.

@TomOnTime TomOnTime requested a review from a team as a code owner March 29, 2020 15:05
@codecov
Copy link

codecov bot commented Mar 29, 2020

Codecov Report

Merging #1098 into master will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
- Coverage   73.18%   73.07%   -0.12%     
==========================================
  Files          33       34       +1     
  Lines        2480     2488       +8     
==========================================
+ Hits         1815     1818       +3     
- Misses        559      564       +5     
  Partials      106      106
Impacted Files Coverage Δ
altsrc/default_input_source.go 0% <0%> (ø)
altsrc/toml_file_loader.go 42.25% <0%> (+0.22%) ⬆️
altsrc/yaml_file_loader.go 43.18% <0%> (+0.32%) ⬆️
altsrc/json_source_context.go 20.32% <0%> (+0.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd51d24...9fd9cd1. Read the comment docs.

docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
Copy link
Member

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

At a high level, can you move the migration guide into it's own file? I would like to see a file structure like this

./docs/migrate-v1-to-v2.md
./docs/v1/manual.md
./docs/v2/manual.md

I think that would be the best option for discoverability here 🙂 I'm imagining a complex far future world that looks like this

./docs/migrate-v1-to-v2.md
./docs/migrate-v1-to-v3.md
./docs/migrate-v2-to-v3.md
./docs/v1/manual.md
./docs/v2/manual.md
./docs/v3/manual.md

And when we're at that point, it will likely make much for sense for the migration guides to be in their own files.

@TomOnTime
Copy link
Contributor Author

Yes, I agree it works better as a separate file. How will people find it? Should I make a link to it in the manual?

@TomOnTime
Copy link
Contributor Author

@lynncyrin I'm unclear how to fix the codecov/project failure. Can I get some assistance?

@coilysiren
Copy link
Member

There should be a link to the migration guide in:

  • the root readme
  • the v1 manual
  • the v2 manual

@coilysiren
Copy link
Member

I don't have any special knowledge about the codecov failure, and it's not really relevant to this PR. You can ignore it.

@TomOnTime
Copy link
Contributor Author

PTAL

Copy link
Member

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

🚢

@coilysiren coilysiren merged commit 7a39010 into urfave:master Apr 6, 2020
@TomOnTime TomOnTime deleted the migration branch April 7, 2020 11:35
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.

Create a v1 => v2 migration guide
4 participants