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

Add custom attribute to flag strings to review #5769

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Oct 6, 2023

This adds a new toReview attribute to strings that we want to review. The idea here is to allow copy discussions to happen separately from PR review: the strings can be reviewed later and then the flag can be removed when we're happy with them.

We're going to experiment with this process on the v2023.3.x and then decide if we want to continue it when merging everything back in to master.

@seadowg seadowg changed the base branch from master to v2023.3.x October 6, 2023 16:55
@@ -9,7 +9,7 @@
the specific language governing permissions and limitations under the
License.
-->
<resources xmlns:tools="http://schemas.android.com/tools" tools:ignore="MissingTranslation" tools:locale="en">
<resources xmlns:tools="http://schemas.android.com/tools" xmlns:oat="http://schema.getodk.org/odkAndroidTools" tools:ignore="MissingTranslation" tools:locale="en">
Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't sure about if we needed an actual like URL for this schema seeing as it's just an experiment for the moment. @lognaturel I think you have a bit more experience with XML schemas than me, so it would be interesting to see what your take is.

Copy link
Member

Choose a reason for hiding this comment

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

It’s really just an opaque identifier so this looks great to me! Oats are tasty.

@seadowg seadowg marked this pull request as ready for review October 6, 2023 16:58
@@ -216,10 +216,6 @@ android {
htmlReport true
lintConfig file("$rootDir/config/lint.xml")
xmlReport true

if (!project.hasProperty("lintStrings")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This workflow makes it much easier to just add strings quickly in a PR (as copy becomes less of a concern) so I don't think we need this.

Copy link
Member

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

Looks like a good experiment to me. @grzesiek2010 we’ll try out having placeholder strings during development and then finalizing them when all the functionality is complete. If we like this process we can decide what to do with Transifex if we use it when developing against master.

@seadowg seadowg merged commit cea35fd into getodk:v2023.3.x Oct 9, 2023
6 checks passed
@seadowg seadowg deleted the to-review branch October 9, 2023 08:56
seadowg added a commit to seadowg/collect that referenced this pull request Oct 17, 2023
Add custom attribute to flag strings to review
seadowg added a commit to seadowg/collect that referenced this pull request Oct 24, 2023
Add custom attribute to flag strings to review
seadowg added a commit to seadowg/collect that referenced this pull request Oct 30, 2023
Add custom attribute to flag strings to review
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.

None yet

2 participants