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

New data model for the exe #55

Merged
merged 1 commit into from
Mar 26, 2016
Merged

New data model for the exe #55

merged 1 commit into from
Mar 26, 2016

Conversation

andreasohlund
Copy link
Owner

There was alot of repetion in the old data model. Spiking a new model that focus more on "api changes" instead of type changes.

  • Removed and internalized type are now in the same collection
  • Changes fields and methods are now in one collection

@andreasohlund
Copy link
Owner Author

The current new format looks like this (obsoletes on individual fields and methods still missing)

------------------- new format ------------------

Breaking changes

The following public types is no longer available

  • Example.MissingNextVersionClass
  • Example.InternalNextVersionClass

The following types have breaking changes

Example.ClassToBeObsoletedWithErrorInNextVersion

Obsoleted with Error - This type should no longer be used, use XYZ instead.

Example.ClassWithMembersToBeObsoletedWithErrorInNextVersion

Example.IMethodChangesParametersNextVersion

Removed methods

  • void MethodName(string)

Example.MemberInternalNextVersion

Removed fields

  • string StringField

Removed methods

  • string get_StringProperty()
  • void Method()
  • void set_StringProperty(string)

Example.MemberMissingNextVersion

Removed fields

  • string StringField

Removed methods

  • string get_StringProperty()
  • void Method()
  • void set_StringProperty(string)

Non breaking changes

Example.ClassToBeObsoletedWithWarnInNextVersion

Obsoleted with Warning - This type should no longer be used, use XYZ instead.
------------------- end new format ------------------

@ParticularLabs/apicomparer what do you all think about the separation of breaking / non breaking changes?

@SeanFeldman
Copy link
Contributor

+1 @andreasohlund

@andreasohlund
Copy link
Owner Author

Give that we have a non breaking section would it make any sense to show "new types" ? cc @SimonCropp

@SimonCropp
Copy link
Contributor

Give that we have a non breaking section would it make any sense to show "new types"

Dont think so

@danielmarbach
Copy link
Contributor

I think that would provide value.

Am 26.12.2015 um 09:38 schrieb Simon Cropp notifications@github.com:

Give that we have a non breaking section would it make any sense to show "new types"

Dont think so


Reply to this email directly or view it on GitHub.

@andreasohlund
Copy link
Owner Author

Had a chat with @SimonCropp and we came to the conclusion that "obsolete with warn" a special form of removal sinces it "notifies of a upcoming apichange". While it's technically a breaking change for users that have treat warnings as errors on we still decided to treat them as non breaking changes since

  1. The implementions should still work
  2. The warning can be suppressed with a compiler directive

I created this mindmap as a result:

https://drive.google.com/a/nservicebus.com/file/d/0Bxjcg5ArjglPSDBSdGJrNlBQSzg/view?usp=sharing

Snapshot:

78f44e508df8013303e83d1857c415ff 1

Thoughts/Ideas/Comments?

@danielmarbach
Copy link
Contributor

Don't we (particular) implement them with throw new NotImplementedException()? Which would be opposed to your brainstorming conclusion?

Am 26.12.2015 um 13:18 schrieb Andreas Öhlund notifications@github.com:

Had a chat with @SimonCropp and we came to the conclusion that "obsolete with warn" a special form of removal sinces it "notifies of a upcoming apichange". While it's technically a breaking change for users that have treat warnings as errors on we still decided to treat them as non breaking changes since

The implementions should still work
The warning can be suppressed with a compiler directive
I created this mindmap as a result:

https://drive.google.com/a/nservicebus.com/file/d/0Bxjcg5ArjglPSDBSdGJrNlBQSzg/view?usp=sharing

Snapshot:

Thoughts/Ideas/Comments?


Reply to this email directly or view it on GitHub.

@andreasohlund
Copy link
Owner Author

Don't we (particular) implement them with throw new NotImplementedException()? Which would be opposed to your brainstorming conclusion?

For "obsolete with error" we do throw but not for "obsolete with warn". Does that make sense?

@danielmarbach
Copy link
Contributor

I might be too tired but as far as I'm involved we mostly used obsolete with error, see

https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/obsoletes.cs

Even for APIs that have been introduced just for one version. Look at the obsoletes. Most of the messages are like: hey we don't compile anymore for V6 and will remove in v7, here is what you should be using.

And the impls throw. So where do we actually use obsolete with warning?

Am 26.12.2015 um 23:06 schrieb Andreas Öhlund notifications@github.com:

Don't we (particular) implement them with throw new NotImplementedException()? Which would be opposed to your brainstorming conclusion?

For "obsolete with error" we do throw but not for "obsolete with warn". Does that make sense?


Reply to this email directly or view it on GitHub.

@SimonCropp
Copy link
Contributor

@danielmarbach that is because we r in the phase of a major release.

@danielmarbach
Copy link
Contributor

Let me rephrase:

How many times have we used obsolete as warn in the past?

Am 27.12.2015 um 00:45 schrieb Simon Cropp notifications@github.com:

@danielmarbach that is because we r in the phase of a major release.


Reply to this email directly or view it on GitHub.

@andreasohlund
Copy link
Owner Author

5.X has a few

http://apicomparer-preview.particular.net/compare/nservicebus/5.0.0...5.1.3

On Sun, Dec 27, 2015 at 8:40 AM, Daniel Marbach notifications@github.com
wrote:

Let me rephrase:

How many times have we used obsolete as warn in the past?

Am 27.12.2015 um 00:45 schrieb Simon Cropp notifications@github.com:

@danielmarbach that is because we r in the phase of a major release.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#55 (comment)
.

@andreasohlund
Copy link
Owner Author

I've got another idea, if we treat "obsolete with warn" as a notification of a upcoming breaking change we get the following layout (note that I added optional parsing of the obsolete string based on knowlege of ObsoleteEx)

------------ start -----------

The following types are no longer available

Example.MissingNextVersionClass

No upgrade instructions provided.

Example.InternalNextVersionClass

No upgrade instructions provided.

Example.ClassToBeObsoletedWithErrorInNextVersion

This type should no longer be used, use XYZ instead.

Types with removed members

Example.ClassWithMembersToBeObsoletedWithErrorInNextVersion

Removed fields

  • string StringField - Use xyz instead.

Removed methods

  • string get_StringProperty() - Can safely be removed.
  • void Method() - This is how to do it.
  • void set_StringProperty(string) - Can safely be removed.

Example.IMethodChangesParametersNextVersion

Removed methods

  • void MethodName(string) - No upgrade instructions provided.

Example.MemberInternalNextVersion

Removed fields

  • string StringField - No upgrade instructions provided.

Removed methods

  • string get_StringProperty() - No upgrade instructions provided.
  • void Method() - No upgrade instructions provided.
  • void set_StringProperty(string) - No upgrade instructions provided.

Example.MemberMissingNextVersion

Removed fields

  • string StringField - No upgrade instructions provided.

Removed methods

  • string get_StringProperty() - No upgrade instructions provided.
  • void Method() - No upgrade instructions provided.
  • void set_StringProperty(string) - No upgrade instructions provided.

The following will be removed in upcoming versions

Version - 2.0.0

Example.ClassToBeObsoletedWithWarnInNextVersion

This type should no longer be used, use XYZ instead. Will be treated as an error from version 2.0.0.

------------ end -----------

@danielmarbach
Copy link
Contributor

Really cool!

Am 28.12.2015 um 11:17 schrieb Andreas Öhlund notifications@github.com:

I've got another idea, if we treat "obsolete with warn" as a notification of a upcoming breaking change we get the following layout (note that I added optional parsing of the obsolete string based on knowlege of ObsoleteEx)

------------ start -----------

The following types are no longer available

Example.MissingNextVersionClass

No upgrade instructions provided.

Example.InternalNextVersionClass

No upgrade instructions provided.

Example.ClassToBeObsoletedWithErrorInNextVersion

This type should no longer be used, use XYZ instead.

Types with removed members

Example.ClassWithMembersToBeObsoletedWithErrorInNextVersion

Removed fields

string StringField - Use xyz instead.
Removed methods

string get_StringProperty() - Can safely be removed.
void Method() - This is how to do it.
void set_StringProperty(string) - Can safely be removed.
Example.IMethodChangesParametersNextVersion

Removed methods

void MethodName(string) - No upgrade instructions provided.
Example.MemberInternalNextVersion

Removed fields

string StringField - No upgrade instructions provided.
Removed methods

string get_StringProperty() - No upgrade instructions provided.
void Method() - No upgrade instructions provided.
void set_StringProperty(string) - No upgrade instructions provided.
Example.MemberMissingNextVersion

Removed fields

string StringField - No upgrade instructions provided.
Removed methods

string get_StringProperty() - No upgrade instructions provided.
void Method() - No upgrade instructions provided.
void set_StringProperty(string) - No upgrade instructions provided.
The following will be removed in upcoming versions

Version - 2.0.0

Example.ClassToBeObsoletedWithWarnInNextVersion

This type should no longer be used, use XYZ instead. Will be treated as an error from version 2.0.0.

------------ end -----------


Reply to this email directly or view it on GitHub.

@andreasohlund
Copy link
Owner Author

Below is the final version of the new layout where also individual member obsoletions is shown with their correct version.

---------------- start new layout -------------

Changes in current version

The following types are no longer available

Example.ClassObsoletedWithWarnShouldBeIncludedIfRemoved

Some message.

Example.MissingNextVersionClass

No upgrade instructions provided.

Example.ClassObsoletedWithWarnShouldBeIncludedIfInternalized

Some message.

Example.InternalNextVersionClass

No upgrade instructions provided.

Example.ClassToBeObsoletedWithErrorInNextVersion

This type should no longer be used, use XYZ instead.

Types with removed members

Example.MemberMissingNextVersion

Removed fields

  • string StringField - No upgrade instructions provided.

Removed methods

  • string get_StringProperty() - No upgrade instructions provided.
  • void Method() - No upgrade instructions provided.
  • void set_StringProperty(string) - No upgrade instructions provided.

Example.IMethodChangesParametersNextVersion

Removed methods

  • void MethodName(string) - No upgrade instructions provided.

Example.ClassWithMembersToBeObsoletedWithErrorInNextVersion

Removed fields

  • string StringField - Use xyz instead.

Removed methods

  • string get_StringProperty() - Can safely be removed.
  • void Method() - This is how to do it.
  • void set_StringProperty(string) - Can safely be removed.

Example.MemberInternalNextVersion

Removed fields

  • string StringField - No upgrade instructions provided.

Removed methods

  • string get_StringProperty() - No upgrade instructions provided.
  • void Method() - No upgrade instructions provided.
  • void set_StringProperty(string) - No upgrade instructions provided.

Upcoming changes in Version - 2.0.0

The following types are no longer available

Example.ClassToBeObsoletedWithWarnInNextVersion

This type should no longer be used, use XYZ instead. Will be treated as an error from version 2.0.0.

Types with removed members

Example.ClassWithMembersToBeObsoletedWithWarnInNextVersion

Removed fields

  • string StringField - Use xyz instead. Will be treated as an error from version 2.0.0.

Removed methods

  • string get_StringProperty() - Can safely be removed. Will be treated as an error from version 2.0.0.
  • void Method() - This is how to do it. Will be treated as an error from version 2.0.0.
  • void set_StringProperty(string) - Can safely be removed. Will be treated as an error from version 2.0.0.

---------------- end new layout -------------

Removals and top level obsoletes working

Only include changed types

Separated breaking and non breaking changes in the markdown formatter

Adding test case for type to be obsoleted with warning

Display obsolete severity

Better distinction between breaking and non breaking changes

Heading tweak

Prepare to add field/method obsoletes

Made headers more terse

Get obsoletes going for fields

Collapsed removed into the a single collection for breaking changes

Together with obsoletes and changed types

Separated removed and changed types again

Better formatting of upgrade instructionBetter formatting of upgrade
instructions

Introduced future versions

Minor tweaks

Display future changes

Separated obsolete with error from warn

Filter out future obsoletes

Using list of apichanges instead

List version properly

Handle changed types

Also separate future member changes

Handle targets that are no longer supported

More cleanup of the obsolete message

Make sure non public member are not included

Do not include non modified internal methods

Dont include not modified internal fields

Do not include members that are already obsoleted with error

Do not include members already obsoleted
@andreasohlund andreasohlund changed the title [WIP] New data model New data model for the exe Mar 26, 2016
@andreasohlund
Copy link
Owner Author

Since this only affects the exe I'm going to merge this. I'll do a different pull for the site once I've tested it IRL for a few weeks

@andreasohlund andreasohlund merged commit 782865e into master Mar 26, 2016
@andreasohlund andreasohlund deleted the new-data-model branch March 26, 2016 09:33
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