Skip to content

Add a v1 => v2 migration guide#1098

Merged
coilysiren merged 12 commits intourfave:masterfrom
TomOnTime:migration
Apr 6, 2020
Merged

Add a v1 => v2 migration guide#1098
coilysiren merged 12 commits intourfave:masterfrom
TomOnTime:migration

Conversation

@TomOnTime
Copy link
Contributor

@TomOnTime TomOnTime commented Mar 29, 2020 •
edited by coilysiren
Loading

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 •
edited
Loading

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.

@TomOnTime TomOnTime mentioned this pull request Mar 29, 2020
Closed
VirrageS
VirrageS suggested changes Mar 30, 2020
View reviewed changes
@coilysiren coilysiren mentioned this pull request Mar 30, 2020
Merged
coilysiren
coilysiren requested changes Mar 31, 2020
View reviewed changes
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

TomOnTime commented Mar 31, 2020

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

TomOnTime commented Apr 1, 2020

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

@coilysiren
Copy link
Member

coilysiren commented Apr 1, 2020

There should be a link to the migration guide in:

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

@coilysiren
Copy link
Member

coilysiren commented Apr 1, 2020

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

TomOnTime commented Apr 4, 2020

PTAL

coilysiren
coilysiren approved these changes Apr 5, 2020
View reviewed changes
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

Reviewers

@saschagrunert saschagrunert saschagrunert approved these changes

@coilysiren coilysiren coilysiren approved these changes

+1 more reviewer

@VirrageS VirrageS VirrageS requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TomOnTime TomOnTime

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Create a v1 => v2 migration guide

4 participants

@TomOnTime @coilysiren @saschagrunert @VirrageS