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

fix: room dependency causing build fail #1324

Merged
merged 1 commit into from
Mar 16, 2019

Conversation

addiegupta
Copy link
Contributor

@addiegupta addiegupta commented Mar 16, 2019

Fixes #1323

Changes:

  1. Changed Room dependency version from version "2.1.0-alpha05"
    to version "2.1.0-alpha04"
  2. Also changed single quotes ' ' to double quotes " " for consistency with other version variables.

EDIT:- Changes made on second commit after Travis build failed on first commit
3) Added the translatable = "false" property to strings that do not have their translations yet so that the Travis build succeeds. When all of their translations have been added, this property can be removed.

The build is now successful

@haroldadmin
Copy link
Contributor

Why are there so many other changes in the strings.xml file?

@addiegupta
Copy link
Contributor Author

So that Travis build passes

@haroldadmin
Copy link
Contributor

haroldadmin commented Mar 16, 2019

I see only one commit in your PR, so how do you know the build fails without them? Are you sure the build fails without these changes?

@addiegupta
Copy link
Contributor Author

addiegupta commented Mar 16, 2019

i first pushed a commit. Travis build failed. Then made the string changes, squashed the commits and then made a force push

@haroldadmin
Copy link
Contributor

haroldadmin commented Mar 16, 2019

No, I meant that if you found out that the build failed without those changes, you would have force pushed another commit to add these changes. Nevermind that, I just wanted to know if you have tried committing without those changes too.

@addiegupta
Copy link
Contributor Author

Yes i did try without the string changes

@haroldadmin
Copy link
Contributor

haroldadmin commented Mar 16, 2019

@iamareebjamal Can you please review this and merge if everything looks good? I want to work on the List Adapter PR but if I commit without these changes then the build will fail again.

@addiegupta
Copy link
Contributor Author

I actually listed the reason for updating the strings on point 3. But I'll make it more clearer. Thanks!

@haroldadmin
Copy link
Contributor

@addiegupta My apologies, I did not spot the description when I read this PR for some reason. It will be great if this PR is merged. Other PRs development can finally continue after this.

@iamareebjamal
Copy link
Member

I don't think the build will fail without the string changes. It'll just throw a warning. However, since this is much needed PR, I'll approve. Create a new PR reverting the changes and let's see if it fails

@iamareebjamal iamareebjamal merged commit 3ab8922 into fossasia:development Mar 16, 2019
@addiegupta
Copy link
Contributor Author

@iamareebjamal okay sure!

@addiegupta addiegupta deleted the #1323 branch March 17, 2019 07:28
addiegupta added a commit to addiegupta/open-event-android that referenced this pull request Mar 17, 2019
Fixes Issue fossasia#1329 . PR fossasia#1324 was merged as it was a required PR but it had some additional unwanted changes which were made so that the Travis build passes. This PR aims to revert these changes and check the status of Travis build
addiegupta added a commit to addiegupta/open-event-android that referenced this pull request Mar 22, 2019
Fixes Issue fossasia#1329 . PR fossasia#1324 was merged as it was a required PR but it had some additional unwanted changes which were made so that the Travis build passes. This PR aims to revert these changes and check the status of Travis build
iamareebjamal pushed a commit that referenced this pull request Mar 22, 2019
Fixes Issue #1329 . PR #1324 was merged as it was a required PR but it had some additional unwanted changes which were made so that the Travis build passes. This PR aims to revert these changes and check the status of Travis build
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.

3 participants