clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed.
Just some header doc cleanup before we submit. Thanks for sticking with these changes! ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:24 + + const lldb::SBEnvironment &operator=(const lldb::SBEnvironment &rhs); + ---------------- const means nothing in these classes since the ivar will never get modified, so you can remote the leading "const" for the return value. ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:27 + /// Return the value of a given environment variable. + /// + /// \return ---------------- need \param entries in header doc ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:29 + /// \return + /// The value of the enviroment variable or null if not present. + const char *Get(const char *name); ---------------- Rephrase the return value docs a bit. Maybe: ``` /// The value of the environment variable or null if not present. If the environment variable has no value but is present a valid pointer to an empty string will be returned. ``` Is that what we are doing? Returning "" when variable is there but has no value? ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:32 + + /// \return + /// The name at the given index or null if the index is invalid. ---------------- need brief description and \param entries in header doc ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:36 + + /// \return + /// The value at the given index or null if the index is invalid. ---------------- need brief description and \param entries in header doc ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:37 + /// \return + /// The value at the given index or null if the index is invalid. + const char *GetValueAtIndex(size_t index); ---------------- Need to clarify what happens when the env far is there but has no value. Should match the Get(const char*) function return value with what ever we say is the solution for that. ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:40 + + size_t GetNumValues(); + ---------------- Move above the *AtIndex(size_t) calls so users see this first in API. ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:44 + /// If the variable exists, its value is updated only if overwrite is true. + /// + /// \return ---------------- need \param entries in header doc ================ Comment at: lldb/include/lldb/API/SBEnvironment.h:50 + /// Unset an environment variable if exists. + /// + /// \return ---------------- need \param entries in header doc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76111/new/ https://reviews.llvm.org/D76111 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
