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.

Sign up for GitHub

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

Jump to bottom

Remove unused code #353

Closed
wants to merge 14 commits into from
Closed

Remove unused code #353

wants to merge 14 commits into from

Conversation

enricosada
Copy link
Contributor

simplify codebase, remove code inside unused directives

each commit is one directive, so easier to rollback or discard

there are lot of directives (160+):

old stuff

obsolete -> remove?

  • OLD_CACHE
  • WORKAROUND_FSHARP_2_0_BUG

features wip/disabled/incomplete

remove or move to branch/pr?

  • DESIGNER
  • TurnWordWrapOnByDefault
  • COLORABLE_ITEM
  • COLORIZE_TYPES

not used?

not only the silvelight stuff

  • FEATURE_PAL
  • !PLATFORM_UNIX
  • __APPLE__

debugger helpers

use a naming convention?

  • DEBUGGERVISUALIZER
  • __DEBUG
  • EXTRA_DEBUG

and others..

forki
forki reviewed Apr 9, 2015
View reviewed changes
src/fsharp/lib.fs
@@ -383,53 +383,6 @@ type Graph<'Data, 'Id when 'Id : comparison and 'Id : equality>
else List.iter (trace (node.nodeData::path)) node.nodeNeighbours
List.iter (fun node -> trace [] node) nodes

#if OLDCODE
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly

dsyme
dsyme reviewed Apr 9, 2015
View reviewed changes
vsintegration/src/vs/FsPkgs/FSharp.Project/Common.Source.CSharp/Project/FileNode.cs
@@ -1122,47 +1067,7 @@ private void RenameCaseOnlyChange(string newFileName)

#endregion

#region SingleFileGenerator Support methods
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about deleting the single-file-generator stuff, someone may want to take a stab at enabling this feature in the project system. I'll let @latkin and @KevinRansom decide

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wip features inside of directives can be moved to a feature branch.

I can:

  • create a branch singleFileGenerator
  • revert 474d880 commit in stage area
  • remove the #if #endif
  • commit

now the singleFileGenerator branch contains all the code active, so easy diff (all code is a single commi) and rebase

If the codebase change, i dont think there is a build matrix with all the directives, maybe some are not aligned or do not compile anymore (this is the problem with conditional directives).
If someone want to continue the work, can easly use this new branch

@dsyme
Copy link
Contributor

dsyme commented Apr 9, 2015

Great work

@dsyme
Copy link
Contributor

dsyme commented Apr 9, 2015

Thank you for doing this - this kind of spring cleaning is so important to maintaining the viability of the codebase going forward, I really appreciate you doing this.

@msftclas
Copy link

@enricosada, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@KevinRansom
Copy link
Member

Thank you for taking the time to take care of this work, we will take care of this at the start of the next cycle.

Moving this to V.Next per: #371

latkin pushed a commit that referenced this pull request Aug 4, 2015
@enricosada @latkin
Remove unused code
51ff4c5 
Deletes code conditional on unused defines
  - SINGLE_FILE_GENERATOR
  - NOT_YET_NEEDED
  - BETA2
  - IMPLEMENT_IVSPERSISTHIERARCHYITEM2
  - IVsOutliningCapableLanguage
  - CUT
  - DISABLED
  - OLDCODE
  - Suggestion4299
  - UNUSED_DEPENDENT_FILES
  - UNUSED_NESTED_PROJECTS
  - NEVER
  - false
  - UNUSED

closes #353

commit 2839aab40f4e3ab434f0738d5c8e3691d7c2c5df
Author: latkin <latkin@microsoft.com>
Date:   Mon Aug 3 17:22:20 2015 -0700

    Fix merge conflict

commit fc4602bf8b60204d85e707db1438f78cc082cb3d
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 18:15:19 2015 +0200

    remove SINGLE_FILE_GENERATOR

commit 2dc0802e76006c6f5dcceae3360325eba01aea6c
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 18:08:21 2015 +0200

    remove NOT_YET_NEEDED

commit df49b75211747dcd4e0ff887ef4473a280bf4a59
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 18:05:56 2015 +0200

    remove BETA2

commit 70858f0689b29638ebfb35e1c3b0aa149895b801
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 18:05:36 2015 +0200

    remove IMPLEMENT_IVSPERSISTHIERARCHYITEM2

commit d5ec2047ed550db9396892bfbdea28b07bc46193
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 18:02:21 2015 +0200

    remove IVsOutliningCapableLanguage

commit 7f78d984171b4ed22da6339c6b799bbd44e743b3
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:55:24 2015 +0200

    remove CUT

commit d781a86012e497dd81fe811250d5efb481c80945
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:54:59 2015 +0200

    remove DISABLED

commit d19d387bd38a0b60e688fe6fa40a24d539d5853d
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:50:06 2015 +0200

    remove OLDCODE

commit 1997a739aa040aa06428f2d225f7d8771b27f938
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:48:02 2015 +0200

    remove Suggestion4299

commit 58cfe73fd9931dfb0276be146dc8d51c04c68c51
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:31:29 2015 +0200

    remove UNUSED_DEPENDENT_FILES

commit 790e49786bb74b1c7a3915ed4b4aaf6243543803
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:19:47 2015 +0200

    remove UNUSED_NESTED_PROJECTS

commit b738f464175e825dad7abdc55ad0e0e20a36545c
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 17:01:08 2015 +0200

    remove NEVER

commit 1f9dea77c2618eeaadec25edced524d51c21b534
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 16:59:43 2015 +0200

    remove false

commit e43fe145b73c81329c1af18744517bef7deb9da4
Author: enricosada <enrico@sada.io>
Date:   Thu Apr 9 16:57:27 2015 +0200

    remove UNUSED
@latkin
Copy link
Contributor

latkin commented Aug 4, 2015

Applied to the OOB branch.

@latkin latkin closed this Aug 4, 2015
@latkin latkin added fixed and removed V.Next labels Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@enricosada @dsyme @msftclas @KevinRansom @latkin @forki