Codecov Report

Merging #2235 (ab67a39) into main (40ac26f) will not change coverage.
The diff coverage is n/a.

❗ Current head ab67a39 differs from pull request most recent head 8ad106b. Consider uploading reports for the commit 8ad106b to get more accurate results

@@           Coverage Diff           @@
##             main    #2235   +/-   ##
=======================================
  Coverage   62.20%   62.20%           
=======================================
  Files         131      131           
  Lines       16772    16772           
=======================================
  Hits        10433    10433           
  Misses       6339     6339           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40ac26f...8ad106b. Read the comment docs.

@jaymedina - what are you trying to do with the last commit? Rerunning the CI with dummy commits without fixing the issue it failed on won't work. The issue was:

/home/docs/checkouts/readthedocs.org/user_builds/astroquery/checkouts/2235/docs/mast/mast.rst:357: WARNING: Inline interpreted text or phrase reference start-string without end-string.

So I would suggest adding the closing backtick that was removed in the second to last commit, and squashing the last 3 together in a rebase. Then all should work and we can merge.

@bsipocz forgot to close one of my strings. Looks like the build is passing now. It turns out the issue wasn't with the second to last commit, it was a string I forgot to close a couple lines below that. Let me know if you need anything else, thanks for looking over this.

Yep, that one wasn't closed in the commit I mentioned.

If you could squash the last three ones it would be great, otherwise it's all looking good.

I tried rebasing and squashing using the documentation here (https://docs.astropy.org/en/latest/development/workflow/development_workflow.html#how-to-squash) and got no errors but nothing seemed to happen in this PR, so I tried running this command which seemed to mess things up locally:

git reset --soft HEAD~3 &&
git commit

Does the squashing need to be done for this PR or can it be merged as is? I'll familiarize myself with these git commands offline before the next PR.

I'll do the squash here then and merge it.

As for practice: My favourite resource both for practice and teach git branch management is https://learngitbranching.js.org/, it has a ton of scenarios (some are very much different than our workflow, but nevertheless, I think it gives a very solid understanding and muscle memory of the underlying logic).

For this case what I would do is (there are other ways, but with rebase you can basically fix any issues as it's not necessary to rebase on the latest main branch, etc.):

• git rebase -i HEAD~5 ---> brings up editor where I would set the last two lines to f for fix-up. Save and exit. There won't be any conflicts.
• git push jaymedina HEAD:mast-cloudaccess-docs-update --force -- > force push back to this branch.

and got no errors but nothing seemed to happen in this PR,

I guess you didn't force push back, it's necessary as there won't be any automated sync between branches, especially if their histories differ, and you have to force push explicitly.

I'll look into this page, thanks very much :)

I did a git push --force origin [this-branch] but for some reason nothing happened after that, so I'm not sure what happened. The only file in the .git/git-rebase/ folder was called git-merge-todo or something like that, which I updated with a simple commit description but that didn't seem to do anything

You basically never need to manually do anything within the .git directory, I guess there might have been another reference, etc so your edit was not picked up. (Also, sometimes you may moved away from the branch locally, then if you push the branch it indeed have them identical (and thus I tend to always push HEAD explicitly into the remote branch)