Skip to content
This repository has been archived by the owner on Jun 28, 2022. It is now read-only.

SampleGen: C# region tag location #2873

Closed
yihanzhen opened this issue Jul 15, 2019 · 3 comments · Fixed by #2886
Closed

SampleGen: C# region tag location #2873

yihanzhen opened this issue Jul 15, 2019 · 3 comments · Fixed by #2886
Assignees
Labels
Core: Sample-gen priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@yihanzhen
Copy link
Contributor

yihanzhen commented Jul 15, 2019

Now, C# samples go like:

// License
// Generated sample, do not edit!

use CommandLine;

// [START region_tag]
namespace Com.Google.Cloud.Language.Samples
{
    import ...;
    import ...;

    public class SampleAnalyzeText
    {
        public static void SampleAnalyzeText(...)
        {
            ...
        }
        // [END region_tag]

        public static void Main()
        {
            // process command line arguments

            SampleAnalyzeText(param1, param2);
        }
    }
}

Start tag and end tag are not at the same level, which will cause samples on devsite to miss closing braces.

Found in: #2872

@yihanzhen
Copy link
Contributor Author

yihanzhen commented Jul 15, 2019

Proposing changes:

// License
// Generated sample, do not edit!

use CommandLine;


namespace Com.Google.Cloud.Language.Samples
{
    // [START region_tag]
    import ...;
    import ...;

    public class SampleAnalyzeText
    {
        public static void SampleAnalyzeText(...)
        {
            ...
        }
    }
    // [END region_tag]

    public class Main
    {
        public static void Main()
        {
            // process command line arguments
            SampleAnalyzeText.SampleAnalyzeText(param1, param2);
        }
    }
}

Since the Main method is just for testing and is not displayed on devsite.

@yihanzhen yihanzhen self-assigned this Jul 15, 2019
@yihanzhen yihanzhen added Core: Sample-gen type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Jul 15, 2019
@yihanzhen
Copy link
Contributor Author

cc/ @beccasaurus @vchudnov-g

@vchudnov-g
Copy link
Contributor

LOL. I just left you a comment in the PR review.

Yes, I really think the region tags should be properly nested within the braces. What you suggest sounds good to me. (Or even just having the tags around the sample function, though that misses the imports, IIUC, which is not as good)

@beccasaurus , wdyt?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core: Sample-gen priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants