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

Feature: Make brm generate more directory aware #137

Closed
Gordonby opened this issue Aug 11, 2022 · 8 comments · Fixed by Azure/bicep#8191
Closed

Feature: Make brm generate more directory aware #137

Gordonby opened this issue Aug 11, 2022 · 8 comments · Fixed by Azure/bicep#8191

Comments

@Gordonby
Copy link
Contributor

Gordonby commented Aug 11, 2022

I've lost count of the number of times that i've done a brm generate in the test folder of a module i'm working on.

Yes i know it's my fault, and i probably should never cd into the test folder.... but you know if brm knows that i'm in a folder called test - it could help me out by asking for confirmation. 😸

@ghost ghost added the Needs: Triage 🔍 Maintainers need to triage still label Aug 11, 2022
@shenglol shenglol added Enhancement and removed Needs: Triage 🔍 Maintainers need to triage still labels Aug 11, 2022
@dciborow
Copy link
Contributor

dciborow commented Aug 12, 2022

I have run brm generate in the root of my repo more times then I can count.

@dciborow
Copy link
Contributor

@shenglol, I know you where not in favor of the automated GitHub approach, but that PR does contain some code which might be helpful for using Git to figure out which directories should updated.

#130

    git status
    # Get path of last file checked in
    pwd
    cd $GITHUB_WORKSPACE
    pwd
    LAST_PATH=$( git log --pretty="" --name-only -n 100 origin/main... -- | fgrep -v ".github" | head -1 )
    echo Last file modified: $LAST_PATH
    # Look for the main template file in this file's path or parents
    SAMPLEFOLDER_PATH=$( dirname "$LAST_PATH" )
    echo Last folder modified: $SAMPLEFOLDER_PATH
    TESTFOLDER_PATH=$SAMPLEFOLDER_PATH
    FOUNDFOLDER_PATH=
    while [ "$TESTFOLDER_PATH" != "." ]
    do
      echo "Looking for main template in $TESTFOLDER_PATH"
      MAINBICEP_PATH=$TESTFOLDER_PATH/main.bicep
      AZDEPLOYJSON_PATH=$TESTFOLDER_PATH/main.json
      if [ -f "$MAINBICEP_PATH" ] || [ -f "$AZDEPLOYJSON_PATH" ]; then
        FOUNDFOLDER_PATH=$TESTFOLDER_PATH
        echo Found main template in $FOUNDFOLDER_PATH
        break
      fi
      TESTFOLDER_PATH=$( dirname "$TESTFOLDER_PATH" )
    done
    if [ ! $FOUNDFOLDER_PATH ]; then
        echo "Could not find main.bicep or azdeploy.json in folder or parents of $SAMPLEFOLDER_PATH" 1>&2
        exit 1
    fi
    echo "SAMPLEFOLDER_PATH=$FOUNDFOLDER_PATH" >> $GITHUB_ENV
    echo "MAINBICEP_PATH=$MAINBICEP_PATH" >> $GITHUB_ENV
    echo "AZDEPLOYJSON_PATH=$AZDEPLOYJSON_PATH" >> $GITHUB_ENV

@dciborow
Copy link
Contributor

I would still like to brainstorm a way that the need to run the command can be eliminated. Once the majority of the work has been completed for a PR, one can not make minor changes without having to return to Codespace or Desktop to regenerate the files.
In azure-quickstarts, we no longer checkin the ARM template, and instead generate it after completion of the PR. As a developer, a similar type of experience would make it easier to iterate when finishing up a PR.

@shenglol
Copy link
Contributor

shenglol commented Aug 17, 2022

I'll find some time this week to working on this and other brm tool improvements.

I would still like to brainstorm a way that the need to run the command can be eliminated. Once the majority of the work has been completed for a PR, one can not make minor changes without having to return to Codespace or Desktop to regenerate the files.
In azure-quickstarts, we no longer checkin the ARM template, and instead generate it after completion of the PR. As a developer, a similar type of experience would make it easier to iterate when finishing up a PR.

@dciborow Actually I thought about this approach before setting up the CI. One thing that's preventing us from implementing it is that each module update PR will end up leaving two commits on the main branch. As a result, the patch version created by nerdbank.gitversioning will always be increased by 2 instead of 1. That being said, I totally agree that having to generate main.json locally is a bit annoying, and it would greatly benefit module contributors if the step can be automated. I'll discuss this with my team and see if we can find a solution. Fow now, one workaround that I think can make our life easier is to set up a local pre-commit git hook to run brm generate automatically before creating a commit.

@dciborow
Copy link
Contributor

@shenglol , one option then is that we can use "git commit --amend && git push --force", to "edit" the commit made by the PR, after it commits to main. This will keep just one commit in the history.

Is the main.json stored in the repo because this is where the bicep module looks it up from? Or is it just for reference.

@shenglol
Copy link
Contributor

We had an internal discussion on git commit --amend && git push --force, and we think that's something we should avoid. Doing force push to main might be a problem because not only does it cause conflicts between the upstream and forked repos, but it also introduces race condition if multiple PRs got merged around the same time.

@Gordonby
Copy link
Contributor Author

For what its worth, I'm not against the json being generated by brm as part of the wider generate process.
Getting into the habit of brm generate and brm validate is OK.

@dciborow
Copy link
Contributor

@shenglol

image

Thank you!!!!

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 a pull request may close this issue.

3 participants