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

chore(release): Add release-only "gitignore bypass" files #1010

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

lucasmcdonald3
Copy link
Contributor

@lucasmcdonald3 lucasmcdonald3 commented Nov 15, 2024

Creates a collection of files that are in .gitignore, but force-added for releases.

As part of release, semantic_release will run git_add_gitignore_bypass_release_files.sh to force add these files.

As part of the "allow local testing" commit, the release technician will run remove_gitignore_bypass_release_files to remove these files. (This is pending an MCM template update. I won't merge this PR until the MCM update is in, but would like to get approval on this PR before merging the MCM update.)

(I don't like the name "release-only 'gitignore bypass'" files, so reviewers should let me know if they have suggestions.)

Testing

  • Release test: Ran make run_semantic_release, it added the ignored files: link
  • Post-release test: Ran sh scripts/release/remove_gitignore_bypass_release_files.sh, it removed these files link

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

josecorella
josecorella previously approved these changes Nov 15, 2024
@lucasmcdonald3 lucasmcdonald3 marked this pull request as ready for review November 18, 2024 17:53
@lucasmcdonald3 lucasmcdonald3 requested a review from a team as a code owner November 18, 2024 17:53
Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

**/*py.dtr
# Ignore everything in ImplementationFromDafny-go...
**/ImplementationFromDafny-go/**
Copy link
Contributor

Choose a reason for hiding this comment

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

You might also want to ignore TestsFromDafny-go ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Should this always be ignored? Or should we also commit this folder on releases?

!**/ImplementationFromDafny-go/**/*extern*
# ... except for go.mod ...
!**/ImplementationFromDafny-go/**/go.mod
Copy link
Contributor

Choose a reason for hiding this comment

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

For Kms and Ddb, we also generate shim.go . But not sure if we want include the shims for other projects (I think we should).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the wrapped shims for testing local services? I think we commit those. (But we haven't released one yet, since the only place we've committed this is in MPL test vectors, which we've never released.)
If those are also just called shim.go, I'll add shim.go to this section

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

# Only proceed if the line is not empty
if [[ -n "$line" ]]; then
# If `git add` doesn't match any files, it errors.
# Swallow error. Release technician should detect issue.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but this comment confused me.
How will the release technician detect an issue if the error is swallowed?

I guess what you are telling me is that this runs before or after some test
that ensures all the critical files are present;
that test's error is not swallowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@lucasmcdonald3 and @texastony, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

@lucasmcdonald3 and @lucasmcdonald3, I noticed you are updating the smithy model files.
Does this update need new or updated user documentation?
Are you adding constraints inside list, map or union? Do you know about this issue: smithy-lang/smithy-dafny#491?

Copy link

Detected changes to the release files or to the check-files action

Copy link

Changes to the release files or the check-files action requires 2 approvals from CODEOWNERS

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 this pull request may close these issues.

6 participants