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 inheritdoc to migration files #10695

Closed
Tracked by #22946
MaklaCof opened this issue Jan 12, 2018 · 12 comments · Fixed by #27268
Closed
Tracked by #22946

Add inheritdoc to migration files #10695

MaklaCof opened this issue Jan 12, 2018 · 12 comments · Fixed by #27268
Assignees
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@MaklaCof
Copy link

It would be nice if dotnet ef migrations add would automatically add

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member

to the beginning of generated file and

#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

at the end of file.
I mean both (migrations and designer).

For example:

#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member

using Microsoft.EntityFrameworkCore.Migrations;
using System;
using System.Collections.Generic;

namespace OpPIS.Web.Hosting.Migrations
{
    public partial class Fix5 : Migration
    {
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.AddColumn<int>(
                name: "ParentAnchorX",
                table: "VarDesign_Parameter-ImageManipulations",
                nullable: true);
        }

        protected override void Down(MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropColumn(
                name: "ParentAnchorX",
                table: "VarDesign_Parameter-ImageManipulations");
        }
    }
}

#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
@ajcvickers
Copy link
Member

@MaklaCof We will have a think about adding XML comments to the generated code, but we don't plan to do it at the present time. Beyond that, we don't want to generate this pragma all the time because most people will not need it. However, there is a similar discussion about auto-generated in issue #10203. I posted a code snippet in that issue showing how to override the generator to add a line--this should also work for the pragma.

@bobvandevijver
Copy link

For reference in this ticket, this is what I used to generate the pragma's:

public class FooMigrationsGenerator : CSharpMigrationsGenerator
{
    private const string PragmaWarningDisable = @"#pragma warning disable 1591";
    private const string PragmaWarningRestore = @"#pragma warning restore 1591";

    public FooMigrationsGenerator(
        MigrationsCodeGeneratorDependencies dependencies, CSharpMigrationsGeneratorDependencies csharpDependencies)
        : base(dependencies, csharpDependencies)
    {
    }

    public override string GenerateMigration(string migrationNamespace, string migrationName,
                                             IReadOnlyList<MigrationOperation> upOperations,
                                             IReadOnlyList<MigrationOperation> downOperations)
    {
        return PragmaWarningDisable
               + Environment.NewLine
               + base.GenerateMigration(migrationNamespace, migrationName, upOperations, downOperations)
               + Environment.NewLine
               + PragmaWarningRestore
               + Environment.NewLine;
    }

    public override string GenerateMetadata(string migrationNamespace, Type contextType, string migrationName,
                                            string migrationId, IModel targetModel)
        => PragmaWarningDisable
           + Environment.NewLine
           + base.GenerateMetadata(migrationNamespace, contextType, migrationName, migrationId, targetModel)
           + Environment.NewLine
           + PragmaWarningRestore
           + Environment.NewLine;
}

Together with this class somewhere in the starting assembly:

public class DesignTimeServices : IDesignTimeServices
{
    /// <inheritdoc />
    public void ConfigureDesignTimeServices(IServiceCollection services)
    {
        // Register custom migration generator which adds a pragma disable for xml-comments
        services.AddSingleton<IMigrationsCodeGenerator, FooMigrationsGenerator>();
    }
}

@MaklaCof
Copy link
Author

MaklaCof commented Feb 2, 2020

This doesn't work anymore with version 3.1.1.
Namespace Microsoft.EntityFrameworkCore.Migrations.Design can not be found.

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 2, 2020

@MaklaCof This? https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dip

@MaklaCof
Copy link
Author

MaklaCof commented Feb 2, 2020

@ErikEJ Does anything else changed?

Class CSharpEntityTypeGenerator is found in using Microsoft.EntityFrameworkCore.Scaffolding.Internal;

But there are no methods: GenerateMigration, GenerateSnapshot, GenerateMetadata.
Only:
image

My Packages are:

    <ItemGroup>
        <PackageReference Include="Autofac" Version="4.8.1" />
        <PackageReference Include="Autofac.Extensions.DependencyInjection" Version="4.3.0" />
        <PackageReference Include="MediatR" Version="5.1.0" />
        <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.SignalR" Version="1.1.0" />
        <PackageReference Include="Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson" Version="3.1.1" />
        <PackageReference Include="Microsoft.AspNetCore.SpaServices.Extensions" Version="3.1.1" />
        <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
        <PackageReference Include="Microsoft.EntityFrameworkCore" Version="3.1.1" />
        <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="3.1.1">
          <PrivateAssets>all</PrivateAssets>
            <!--
            Without this, Migrations.cs file doesn't compile.
            https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/breaking-changes#dip
            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
          -->
        </PackageReference>
        <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="3.1.1">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
        <PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="3.1.0" />
        <PackageReference Include="OpenIddict" Version="2.0.1" />
        <PackageReference Include="OpenIddict.EntityFrameworkCore" Version="2.0.1" />
        <PackageReference Include="OpenIddict.Mvc" Version="2.0.1" />
        <PackageReference Include="Swashbuckle.AspNetCore" Version="4.0.1" />
        <PackageReference Include="Z.EntityFramework.Extensions.EFCore" Version="3.0.26" />
        <PackageReference Include="Z.EntityFramework.Plus.EFCore" Version="3.0.29" />
    </ItemGroup>

@ErikEJ
Copy link
Contributor

ErikEJ commented Feb 2, 2020

Possibly, since you are using Internal classes

@Ablinne
Copy link

Ablinne commented May 8, 2020

I think this should really be implemented. The counter-arguments

  • Most people do not need it
  • Documentation comments should eventually be added

are imho invalid, because:

  • It is just a single line or two that are helpful to everyone who uses xml documentation and visual studio but do not cost anything for everybody else.
  • When documentation comments are eventually added, these pragmas can also be easily removed again. This issue was opened over two years ago!

@roji
Copy link
Member

roji commented Aug 11, 2020

Opening to re-discuss in triage.

@ajcvickers
Copy link
Member

This does seem reasonable to do. We should consider using inheritdoc.

@roji roji self-assigned this Aug 14, 2020
@ajcvickers ajcvickers changed the title Suggestion: Add pragma disabled CS1591 to migration files Add doc comments to migration files, or disable CS1591 Oct 10, 2020
@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Oct 22, 2021
@ajcvickers
Copy link
Member

Note from triage: use inheritdoc for this.

@roji roji changed the title Add doc comments to migration files, or disable CS1591 Add inheritdoc to migration files Oct 23, 2021
@jeancallisti
Copy link

Please do consider this for next release. XML comments are not the only ones causing annoying warnings.

Each new Migration file triggers several warnings. I need to disable at least those manually:
#pragma warning disable CA1814 // Prefer jagged arrays over multidimensional
#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type.

CA1814 is mostly caused by seeding using "HasData"
CS8625 is caused by the use of anonymous types when creating nullable Owned entities.

@roji
Copy link
Member

roji commented Oct 25, 2021

@jeancallisti CS8625 (nullability) is covered by #18950. But I'll look into CA1814 when doing this.

roji added a commit to roji/efcore that referenced this issue Jan 24, 2022
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 24, 2022
roji added a commit to roji/efcore that referenced this issue Jan 24, 2022
And suppress CA1814 if multidimensional seed data is present.

Closes dotnet#10695
roji added a commit to roji/efcore that referenced this issue Jan 24, 2022
And suppress CA1814 if multidimensional seed data is present.

Closes dotnet#10695
roji added a commit to roji/efcore that referenced this issue Jan 25, 2022
And suppress CA1814 if multidimensional seed data is present.

Closes dotnet#10695
roji added a commit to roji/efcore that referenced this issue Jan 25, 2022
And suppress CA1814 if multidimensional seed data is present.

Closes dotnet#10695
roji added a commit that referenced this issue Jan 28, 2022
And suppress CA1814 if multidimensional seed data is present.

Closes #10695
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1, 7.0.0-preview2 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants