-
-
Notifications
You must be signed in to change notification settings - Fork 474
Add Briefcase-specific testing section to write_code docs #2615
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
Conversation
|
@freakboy3742 I’ve added the requested Jinja block scaffold in |
|
You've got the right place - however, as you may have noticed from the CI pass, the escaping you've added for |
|
Thanks for pointing that out. The escaping and towncrier issues are fixed and CI is now passing. I’ll add the Briefcase-specific content only within the "testing_additional" block, keeping the rest unchanged. Please let me know if that works. |
| @@ -1,5 +1,3 @@ | |||
| # Writing, running, and testing code | |||
|
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why have you deleted the title from the document?
freakboy3742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build is now succeeding; however, as noted inline, you've deleted the title from the page. The title for the new section is at level 2, rather than level 3; and there's also a lot of extra blank lines - one line between statements is more than enough.
The ReadTheDocs CI job gives you a sample rendering of what the page currently looks like; that's worth checking out in addition to your own local build.
However, other than those changes, it's now ready for you to add new content.
|
|
||
| {% block testing_additional %} | ||
|
|
||
| ## Briefcase-specific content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Briefcase-specific content | |
| ### Briefcase-specific content |
|
I’ve pushed the requested structural fixes (restored the title, adjusted the heading level to ###, and cleaned up spacing). Just a quick note: I have been unable to successfully run tox -e docs on my local Windows machine with Python 3.12. The build consistently fails during the environment setup due to a dependency issue. The failure appears to be related to the installation of the tzdata dependency within the tox virtual environment on the Windows platform. This seems to be a common environment-specific conflict between tzdata and certain versions of Python/Windows. Since the docs CI is passing, I’m relying on CI as the source of truth for now. Please let me know if you’d prefer a different approach. |
|
@kshivani06 You're on the right track. You can go ahead and start adding content, if you like. I want to let you know that the core team is currently on holiday leave, so response times are going to be slower for the next few weeks. You can likely expect a response within eight days, possibly sooner, as we'll be checking in weekly. We appreciate your patience. |
|
Hi , I’ve added all the requested documentation content covering:
The remaining CI failure appears to be unrelated to the documentation content and is occurring during the docs build due to missing shared macro templates in the docs tooling, affecting multiple existing pages. Please let me know if you’d like any adjustments to the content, additional clarification, or further sections added. I’m happy to update this as needed. Thanks! |
|
@kshivani06 Hello. A reminder that the team is on holiday leave, so it'll be a bit before we can respond. However, before we can review, the PR needs to be passing CI. The documentation failures are because you need to merge |
|
"Hi @kattni, thank you for the guidance! I have successfully merged upstream/main into my branch and resolved the filename conflicts. The documentation build is now passing. Happy holidays to the team!" |
freakboy3742
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great first draft! I've put some brief comments inline; but I've also pushed an update that addresses them, as it's a little easier to make the changes directly rather than suggest them in GitHub.
Thanks for the PR - I'll get another set of eyeballs to check this to make sure I haven't introduced any extra typos; but otherwise, this is a great update to the docs.
|
|
||
| When contributing changes to templates, it is often necessary to test those changes locally before they are merged. The following sections describe how to configure Briefcase to use modified template repositories and branches for testing purposes. | ||
|
|
||
| ### Testing wizard template changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be at level 4 so that there's some structure to this section about templates.
|
|
||
| 1. Fork and clone the wizard template repository you are working on. | ||
| 2. Make your changes in a local branch. | ||
| 3. Run `briefcase new`, specifying the custom template repository and branch: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Local file usage is also important here - possibly more important for local testing purposes.
| 3. Open the project’s `pyproject.toml` file. | ||
| 4. In the platform-specific configuration section, set the template repository and branch. For example: | ||
|
|
||
| ```toml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, local file changes are also important.
| The `template` configuration option specifies the Git repository containing the platform template. | ||
| The `template_branch` configuration option specifies the branch of that repository that Briefcase should use when rendering the template. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can reference the configuration options directly with [template][] syntax.
|
|
||
| 1. Create a branch containing the required changes in the Briefcase repository. | ||
| 2. Create a corresponding branch in the template repository that depends on those changes. | ||
| 3. Configure the test project to use both the modified Briefcase code and the modified template when testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't needed - this is what briefcase-repo: and briefcase-ref: are for.
kattni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @kshivani06 for the contribution!
@freakboy3742 Your final updates look good to me.
All issues have been addressed.
|
Thank you for the review and guidance |
This PR adds a Briefcase-specific documentation block to
docs/en/how-to/contribute/how/write_code.mdusing thetesting_additionalJinja block, as discussed in #2238.This establishes the structure and placement for
Briefcase-specific contribution and testing guidance.
The content will be expanded once the structure is
confirmed.
Feedback welcome.