blender/blender-developer-docs
7
8
Fork 38
Code Issues 3 Pull Requests 6 Activity

Update "Commiting Test Data" section to include build bot testing #77

Merged
Alaska merged 8 commits from Alaska/blender-developer-docs:test-updates-guide into main 2024-10-07 14:56:31 +02:00
Conversation 2 Commits 8 Files Changed 1 +43 -1
Alaska commented 2024-10-02 11:19:23 +02:00
Member
Copy Link

This commit updates the "Commiting Test Data" section to include
information on how to test that your tests are working using the
build bot before commiting the changes to main.

This commit updates the "Commiting Test Data" section to include information on how to test that your tests are working using the build bot before commiting the changes to main.
❤️ 1
Alaska added 1 commit 2024-10-02 11:19:25 +02:00 
This commit updates the "Commiting Test Data" section
to include information on how to test that your tests are working
using the build bot before commiting the changes to main
Alaska commented 2024-10-02 11:19:46 +02:00
Author
Member
Copy Link

Currently WIP due to the TODO. Input from others would be helpful here.

Currently WIP due to the TODO. Input from others would be helpful here.
Alaska requested review from Sergey Sharybin 2024-10-02 11:19:54 +02:00
Sergey Sharybin reviewed 2024-10-02 11:36:00 +02:00
docs/handbook/testing/setup.md Outdated
@ -165,2 +176,2 @@
cd ../..
git push origin main
TODO: Add commands?
I personally haven't done merging/squishing of branches into main so I'm not sure the correct approach here.
Sergey Sharybin commented 2024-10-02 11:35:56 +02:00
Owner
Copy Link

There should be something like

git checkout main
git merge --ff-only add-x-tests
git push origin main

And then land the PR from the Gita interface.
Maybe would be good to have @weizhen, @Sirgienko, or @LukasStockner have a look. They were working with tests more recently than me.

There should be something like ``` git checkout main git merge --ff-only add-x-tests git push origin main ``` And then land the PR from the Gita interface. Maybe would be good to have @weizhen, @Sirgienko, or @LukasStockner have a look. They were working with tests more recently than me.
Alaska requested review from Lukas Stockner 2024-10-02 11:37:11 +02:00
Alaska requested review from Weizhen Huang 2024-10-02 11:37:11 +02:00
Alaska added 3 commits 2024-10-02 11:57:24 +02:00
Weizhen Huang approved these changes 2024-10-02 13:06:02 +02:00
Dismissed
Weizhen Huang left a comment
Member
Copy Link

The steps looks correct to me, but does it need to be a standard approach? My thought was to have an additional subsection, for people who need/want to guarantee that the tests pass. It might not always be necessary.

The steps looks correct to me, but does it need to be a standard approach? My thought was to have an additional subsection, for people who need/want to guarantee that the tests pass. It might not always be necessary.
docs/handbook/testing/setup.md Outdated
@ -158,1 +161,4 @@
git checkout -B add-x-tests-blender
git commit sourcefile1 sourcefile2 tests/data
git push me add-x-tests-blender
# then create a pull request for this branch
Weizhen Huang commented 2024-10-02 13:02:48 +02:00
Member
Copy Link

This should not be part of the code block, but a paragraph, preferably linked to https://developer.blender.org/docs/handbook/contributing/pull_requests/#create-a-pull-request

This should not be part of the code block, but a paragraph, preferably linked to https://developer.blender.org/docs/handbook/contributing/pull_requests/#create-a-pull-request
Alaska marked this conversation as resolved
docs/handbook/testing/setup.md Outdated
@ -159,3 +165,3 @@
```
Check everything is ok, and push to both repositories.
Ensure your tests pass on the build bot commenting `@blender-bot build`,
Weizhen Huang commented 2024-10-02 13:05:14 +02:00
Member
Copy Link

Should be in the same paragraph as "create a pull request". We can also add a link to the bot https://projects.blender.org/infrastructure/blender-bot/src/branch/main/README.md

Should be in the same paragraph as "create a pull request". We can also add a link to the bot https://projects.blender.org/infrastructure/blender-bot/src/branch/main/README.md
Alaska marked this conversation as resolved
docs/handbook/testing/setup.md Outdated
@ -166,0 +179,4 @@
git push origin main
```
Preferably straight after pushing the updated tests, update
Weizhen Huang commented 2024-10-02 12:53:18 +02:00
Member
Copy Link

It's more straightforward to merge your PR from gitea, that would also close the PR.

It's more straightforward to merge your PR from gitea, that would also close the PR.
Alaska marked this conversation as resolved
Weizhen Huang requested changes 2024-10-02 13:06:18 +02:00
Dismissed
Weizhen Huang left a comment
Member
Copy Link

Wrong button

Wrong button
Alaska added 3 commits 2024-10-02 15:44:19 +02:00
Alaska commented 2024-10-02 15:46:05 +02:00
Author
Member
Copy Link

I've made the requested changes and split the build bot testing out into a subsection.

A lot of information is repeated in the subsection, and I'm having a difficult time figure out what I can remove to compact it, while keeping the steps as clear as possible.

If you're okay with repeated information, then say so, if not feel free to give some suggestions on what could be removed.

I've made the requested changes and split the build bot testing out into a subsection. A lot of information is repeated in the subsection, and I'm having a difficult time figure out what I can remove to compact it, while keeping the steps as clear as possible. If you're okay with repeated information, then say so, if not feel free to give some suggestions on what could be removed.
Weizhen Huang added 1 commit 2024-10-07 12:16:41 +02:00
Weizhen Huang approved these changes 2024-10-07 12:17:22 +02:00
Weizhen Huang left a comment
Member
Copy Link

I made some changes, mainly

  • Some information are repeated in the code block and in the text, I have removed them from the text.
  • A line starting with # should be interpreted as a comment of the next line, not additional steps that need to be done, so I have reverted the .. change. It's not very clear but I think it's better than ambiguous syntax.
  • Removed the code block regarding merging the PR, it is enough to merge from Gitea and too many alternatives might be confusing

The repeated information with the main section is fine with me. I also changed some wording, if it reads well to me we can push it.

I made some changes, mainly - Some information are repeated in the code block and in the text, I have removed them from the text. - A line starting with `#` should be interpreted as a comment of the next line, not additional steps that need to be done, so I have reverted the `..` change. It's not very clear but I think it's better than ambiguous syntax. - Removed the code block regarding merging the PR, it is enough to merge from Gitea and too many alternatives might be confusing The repeated information with the main section is fine with me. I also changed some wording, if it reads well to me we can push it.
👍 1
Weizhen Huang changed title from WIP: Update "Commiting Test Data" section to include build bot testing to Update "Commiting Test Data" section to include build bot testing 2024-10-07 12:17:32 +02:00
Sergey Sharybin approved these changes 2024-10-07 14:11:40 +02:00
Sergey Sharybin left a comment
Owner
Copy Link

Seems good. Thanks for the initiative for writing the info down, and iterating on the wording!
Think we can just land the PR and re-iterate on whatever we might have missed as a follow-up?

Seems good. Thanks for the initiative for writing the info down, and iterating on the wording! Think we can just land the PR and re-iterate on whatever we might have missed as a follow-up?
Alaska merged commit 1929528ba0 into main 2024-10-07 14:56:31 +02:00
Alaska deleted branch test-updates-guide 2024-10-07 14:56:31 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Lukas Stockner
Weizhen Huang
Sergey Sharybin
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-developer-docs#77
No description provided.
Delete Branch "Alaska/blender-developer-docs:test-updates-guide"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?