Skip to content

Conversation

@bhardwajs
Copy link
Contributor

Without cloning submodules, build process as outlined fails. Amend README to add recurse_submodules to git clone.

Description

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

  • Identifiers in product code changes are properly _Ugly as per
    https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
  • The STL builds successfully and all tests have passed (must be manually
    verified by an STL maintainer before automated testing is enabled on GitHub,
    leave this unchecked for initial submission).
  • These changes introduce no known ABI breaks (adding members, renaming
    members, adding virtual functions, changing whether a type is an aggregate
    or trivially copyable, etc.).
  • These changes were written from scratch using only this repository,
    the C++ Working Draft (including any cited standards), other WG21 papers
    (excluding reference implementations outside of proposed standard wording),
    and LWG issues as reference material. If they were derived from a project
    that's already listed in NOTICE.txt, that's fine, but please mention it.
    If they were derived from any other project (including Boost and libc++,
    which are not yet listed in NOTICE.txt), you must mention it here,
    so we can determine whether the license is compatible and what else needs
    to be done.

Without cloning submodules, build process as outlined fails. Amend README to add `recurse_submodules` to `git clone`.
@bhardwajs bhardwajs requested a review from a team as a code owner March 28, 2020 00:00
@cbezault
Copy link
Contributor

I just mentioned in #647 (comment) that I created a hard requirement on the llvm-project submodule in #520 which did not previously exist. I want to go modify the CMakeLists.txt to only take that dependency if testing is enabled.

@cbezault cbezault added the documentation Related to documentation or comments label Mar 28, 2020
@bhardwajs
Copy link
Contributor Author

@cbezault

I just mentioned in #647 (comment) that I created a hard requirement on the llvm-project submodule in #520 which did not previously exist. I want to go modify the CMakeLists.txt to only take that dependency if testing is enabled.

Seems like the best course of action would be to make changes in README as part of #647. What do you think? If you agree, I will abandon this PR.

@cbezault
Copy link
Contributor

Yes there does seem to be a need for a change in the README related to this in #647 but I also need to open a new PR which resolves the fact that I added an unnecessary dependency.

done this before, you may be prompted to elevate.
7. Open Visual Studio, and choose the "Clone or check out code" option. Enter the URL to this
repository, typically `https://github.com/microsoft/STL`
repository, typically `https://github.com/microsoft/STL`. Make sure that submodules are also checked out.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a checkbox or something for this? If not we need an extra step that explains how to do that.

6. Open an "x64 Native Tools Command Prompt for VS 2019".
7. Change directories to a location where you'd like a clone of this STL repository.
8. Invoke `git clone https://github.com/microsoft/STL`
8. Invoke `git clone --recurse-submodules https://github.com/microsoft/STL`
Copy link
Member

Choose a reason for hiding this comment

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

I think this should have only the instructions for the submodule necessary to build since bringing in this vcpkg copy might not be relevant for folks with an ambient vcpkg available

@bhardwajs
Copy link
Contributor Author

@BillyONeal - based on the thread with @cbezault above, I will pause this PR and follow #647 . If everything is captured in #647 or follow-up PRs, I will close this PR. Thanks for looking at it.

@cbezault
Copy link
Contributor

I think #647 will address everything this PR is trying to address.

@bhardwajs
Copy link
Contributor Author

Thanks @cbezault. Closing the PR.

@bhardwajs bhardwajs closed this Mar 31, 2020
@bhardwajs bhardwajs deleted the sumitb/readme_recurse branch May 4, 2020 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Related to documentation or comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants