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 old profile reminders #591

Closed
wants to merge 3 commits into from
Closed

Conversation

liujian1368928
Copy link

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.

  • How to fix?


New feature or improvement

提示开发 升级到2.2 版本 没有更改配置文件名称导致 失效问题

@@ -15,7 +15,7 @@
</ItemGroup>

<ItemGroup>
<Content Update="skyapm.json">
Copy link
Member

Choose a reason for hiding this comment

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

We can't use skywalking here. skywalking belongs to ASF, we don't donate this agen into ASF.


namespace Microsoft.Extensions.DependencyInjection
{
public static class ServiceCollectionExtensions
{
private static string oldConfigName = "skywalking.json";
Copy link
Member

Choose a reason for hiding this comment

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

When did we use this name? From which version?

Copy link
Author

Choose a reason for hiding this comment

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

skywalking.json???

Copy link
Member

Choose a reason for hiding this comment

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

Yes. 2.1 to 2.2?

Copy link
Author

Choose a reason for hiding this comment

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

It used to be this sywalk.json, I used the old version to upgrade to 2.2, but apache-skywalk-APM-9.5.0 view is empty, but I have dropped to 2.1, view the source code only to find that the configuration name has changed

@wu-sheng
Copy link
Member

Please rename the title of PR to English.

@wu-sheng wu-sheng added this to the 2.3.0 milestone Apr 25, 2024
@@ -2,7 +2,7 @@
<PropertyGroup>
<GeneratePackageOnBuild>True</GeneratePackageOnBuild>
<TargetFramework>net7.0</TargetFramework>
<Nullable>enable</Nullable>
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

Copy link
Author

Choose a reason for hiding this comment

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

@liujian1368928 liujian1368928 changed the title 20240425 添加老配置文件提醒 20240425 Add old profile reminders Apr 25, 2024
@wu-sheng wu-sheng changed the title 20240425 Add old profile reminders Add old profile reminders Apr 25, 2024
@liuhaoyang
Copy link
Collaborator

liuhaoyang commented Apr 26, 2024

builder.AddJsonFile("skywalking.json", true)

Now it still supports config file to be named as skywalking.json

@wu-sheng
Copy link
Member

@liuhaoyang tole me this is a bug introduced by a previous commit. Let's wait for him about what should be the better fix.

@liujian1368928
Copy link
Author

Sorry, I apologize for my lack of rigor. After careful testing, there is no problem

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.

3 participants