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

Replaced /mybucket/ with amzn-s3-demo-bucket #9220

Conversation

DavidSouther
Copy link
Contributor

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@elysahall elysahall self-assigned this Jan 16, 2025
@elysahall elysahall added documentation This is a problem with documentation. pr:work-in-progress This PR is a draft and needs further work. labels Jan 16, 2025
Copy link
Collaborator

@elysahall elysahall left a comment

Choose a reason for hiding this comment

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

LGTM, sending for maintainer review.

@elysahall elysahall added pr:needs-review This PR needs a review from a Member. and removed pr:work-in-progress This PR is a draft and needs further work. labels Jan 21, 2025
@elysahall elysahall assigned kdaily and unassigned elysahall Jan 21, 2025
@kdaily
Copy link
Member

kdaily commented Jan 23, 2025

Due to other recently merged PRs, these have some conflicts to fix.

@DavidSouther
Copy link
Contributor Author

Update merged.

@kdaily kdaily closed this Feb 26, 2025
@kdaily kdaily reopened this Feb 26, 2025
@kdaily
Copy link
Member

kdaily commented Feb 26, 2025

@DavidSouther - thanks for the update, and apologies for the delay in getting the tests to re-run; when there are force pushes GitHub does not always force the test suite to re-run.

Unfortunately the changes have some formatting errors - you can see in the test suite, but they look like:

    def verify_is_valid_rst(filename):
        _, errors = parse_rst(filename)
        if errors:
>           raise AssertionError(_make_error_msg(filename, errors))
E           AssertionError: The file "s3/cp.rst" contains invalid RST: 
E           
E           Line 88: Literal block expected; none found.
E             
E           
E           Line 89: Inline emphasis start-string without end-string.
E             aws s3 cp s3://amzn-s3-demo-bucket/ s3://amzn-s3-demo-bucket2/ \

functional/docs/test_examples.py:179: AssertionError

https://github.com/aws/aws-cli/actions/runs/13550995902/job/37874307218?pr=9220#step:5:521

@kdaily kdaily assigned DavidSouther and kdaily and unassigned kdaily Feb 26, 2025
@DavidSouther DavidSouther force-pushed the mybucket-MM7GNG7ZXSIQE_to_amzn-s3-demo-bucket-YKOD556SZIQ2U branch from b8deb99 to 2935fc0 Compare March 5, 2025 21:25
@DavidSouther
Copy link
Contributor Author

Oops, I missed your comment about force pushes not reruning tests. I typically do squash-rebase PRs to keep them tidy.

Updated to latest tip, but that error looks spurious to me - the block has the same indentation and formatting as previously, and the block shouldn't be checking for inline styles?

@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.08%. Comparing base (810493f) to head (2935fc0).
Report is 9 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #9220   +/-   ##
=======================================
  Coverage     0.08%   0.08%           
=======================================
  Files          210     210           
  Lines        16984   16984           
=======================================
  Hits            14      14           
  Misses       16970   16970           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kdaily
Copy link
Member

kdaily commented Mar 5, 2025

Strange, just ran the tests locally and they passed. I just approved the GitHub CI to run again so hopefully everything is good now. Thanks!

@kdaily
Copy link
Member

kdaily commented Mar 6, 2025

Waiting for tests to pass on the CLI v2 port of this PR: #9347

@kdaily kdaily merged commit f13cfc2 into aws:develop Mar 6, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation. pr:needs-review This PR needs a review from a Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants