Skip to content

Commit missing built asset files #589

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Commit missing built asset files #589

wants to merge 1 commit into from

Conversation

stof
Copy link
Member

@stof stof commented Jun 14, 2017

Assets are meant to be committed, to make the demo work out of the box without having to run Encore. But this works only if all files gets committed. Committing only 2 of 15 files is broken.

@stof
Copy link
Member Author

stof commented Jun 14, 2017

Btw, there is not contributing doc about how to build assets AFAIK (we need to tell people to run ./node_modules/.bin/encore production before committing to ship with the working version)

@stof
Copy link
Member Author

stof commented Jun 14, 2017

@javiereguiluz I will rebase this PR and rebuild files, as #587 has been merged.

@stof stof force-pushed the missing_assets branch from a73c960 to 3435c2d Compare June 14, 2017 09:41
@stof
Copy link
Member Author

stof commented Jun 14, 2017

@javiereguiluz done

@javiereguiluz
Copy link
Member

@weaverryan please merge this PR because for some unknow reason I can't do that:

<error>Unable to merge branch pull/589</error>.
Unable to merge branch pull/589
Resolve the conflict by hand and start the command again

I can't see any conflict after that message, so I can't solve nothing ... but if I don't solve something, it doesn't let me merge this 😡 😡 😡

@bocharsky-bw
Copy link
Contributor

@javiereguiluz Sounds like a bug in your internal tool ;) Maybe you need to make a meanless commit after this error message and try to merge again?

@stof
Copy link
Member Author

stof commented Jun 15, 2017

@javiereguiluz I think you may have the non-committed build files locally, which prevent the merge because the merge would have to overwrite untracked files.
Run a git clean on your project to delete untracked asset files and start again

@javiereguiluz
Copy link
Member

I've tried git clean -f but it doesn't work. I'm going to report the error in the repo of the tool we use to merge things. Thanks.

@stof
Copy link
Member Author

stof commented Jun 15, 2017

@javiereguiluz don't use the GH resume feature either. It was not made for the case where the merge is rejected entirely (which fails earlier in git)

@javiereguiluz javiereguiluz mentioned this pull request Jun 19, 2017
Merged
javiereguiluz added a commit that referenced this pull request Jun 19, 2017
@javiereguiluz
minor #593 Commit missing built asset files (javiereguiluz)
65d0665 
This PR was merged into the master branch.

Discussion
----------

Commit missing built asset files

This is the same as #589 but hopefully this one will be mergeable.

Commits
-------

ae9001c Commit missing built asset files
@javiereguiluz
Copy link
Member

Closing it in favor of #593. @stof I'm sorry but I couldn't merge your PR in any way.

@javiereguiluz javiereguiluz mentioned this pull request Jun 19, 2017
Closed
@stof stof deleted the missing_assets branch May 12, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weaverryan weaverryan weaverryan approved these changes

@alOneh alOneh alOneh approved these changes

@ogizanagi ogizanagi ogizanagi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stof @javiereguiluz @bocharsky-bw @weaverryan @alOneh @ogizanagi