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

Jump to bottom

Update submodule url for .gitmodules #2531

Merged
merged 1 commit into from Apr 28, 2019
Merged

Update submodule url for .gitmodules #2531

merged 1 commit into from Apr 28, 2019

Conversation

SataQiu
Copy link
Contributor

@SataQiu SataQiu commented Apr 26, 2019

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.
    As our code repository has changed its name, the corresponding reference url should be updated.

  • How to fix?
    Update submodule url for .gitmodules.

New feature or improvement

  • Describe the details and related test reports.

wu-sheng
wu-sheng approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

If you want to change, should be fine. 😜 The old ones work too.

@wu-sheng wu-sheng added this to the 6.1.0 milestone Apr 26, 2019
@wu-sheng wu-sheng added the feature New feature label Apr 26, 2019
@coveralls
Copy link

coveralls commented Apr 26, 2019 •
edited

Coverage Status

Coverage increased (+0.002%) to 16.015% when pulling 77507bb on SataQiu:fix-gitmodules-20190427 into 634a7fd on apache:master.

wu-sheng
wu-sheng requested changes Apr 26, 2019
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

From the GitHub preview, the submodules have not been updated.

@wu-sheng
Copy link
Member

Check your repo, https://github.com/SataQiu/incubator-skywalking/tree/fix-gitmodules-20190427/apm-protocol/apm-network/src/main

It still shows in old link. You need to follow git document, remove gitsubmodule first(several steps), then add new one.
I assume you edit the submodule file directly

@wu-sheng wu-sheng removed this from the 6.1.0 milestone Apr 26, 2019
@wu-sheng wu-sheng added TBD To be decided later, need more discussion or input. and removed feature New feature labels Apr 26, 2019
@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 26, 2019

Thanks a lot @wu-sheng .
I am not very familiar with git submodule. The code has been updated.
Clould you please have a look?

@wu-sheng
Copy link
Member

Yes. Submodule updated right. But you submit an unnecessary file. Please remove it.

@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 26, 2019

I don't know, but it seems to be back to the first submission 😭 , how to deal with it?
@wu-sheng

@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 26, 2019

Is that right?

@wu-sheng wu-sheng added feature New feature and removed TBD To be decided later, need more discussion or input. labels Apr 26, 2019
@wu-sheng wu-sheng added this to the 6.1.0 milestone Apr 26, 2019
wu-sheng
wu-sheng requested changes Apr 26, 2019
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Check from local, submodule doesn't update.

@wu-sheng
Copy link
Member

Look like right :)

wu-sheng
wu-sheng previously approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Yes. You updated right. Confirmed now.

@wu-sheng
Copy link
Member

I had been misguided by GitHub page. @SataQiu Could you please update the submodule, and add *.pb.html again? Then I could merge.

Sorry, ask you to do wrong adjustments.

wu-sheng
wu-sheng previously approved these changes Apr 26, 2019
View reviewed changes
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM.

@wu-sheng
Copy link
Member

I will merge this tomorrow.

@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 26, 2019

OK. Thank you! 😆

@wu-sheng
Copy link
Member

See RAT report in: /home/travis/build/apache/skywalking/apm-protocol/apm-network/target/rat.txt

Based on this, I assume because we need another PR to service-mesh-probe/istio/skywalking.config.pb.html in the protocol repo.

We need to add Apache license header, and update submodule again.

@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 27, 2019

ok, I would love to do it.

@wu-sheng wu-sheng removed this from the 6.1.0 milestone Apr 27, 2019
@wu-sheng wu-sheng added this to the 6.2.0 milestone Apr 27, 2019
@SataQiu SataQiu mentioned this pull request Apr 27, 2019
Merged
@SataQiu
Copy link
Contributor Author

SataQiu commented Apr 28, 2019

The CI passed!
I think we can merge it.
@wu-sheng

@wu-sheng wu-sheng modified the milestones: 6.2.0, 6.1.0 Apr 28, 2019
@wu-sheng wu-sheng merged commit 4862c4f into apache:master Apr 28, 2019
@wu-sheng
Copy link
Member

Merged. 6.1 will be cut soon.

@KangZhiDong KangZhiDong mentioned this pull request Apr 28, 2019
Merged
wu-sheng pushed a commit that referenced this pull request Apr 28, 2019
@KangZhiDong @wu-sheng
Update all "incubator-skywalking" to "skywalking" ,"incubator-service…
2dcf27b 
…comb-java-chassis" to "servicecomb-java-chassis" correctly, and ".gitmodules" will update at #2531 (#2548)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wu-sheng wu-sheng wu-sheng approved these changes

Assignees
No one assigned
Labels
feature New feature
Projects
None yet
Milestone
6.1.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SataQiu @coveralls @wu-sheng