search

Obsidian-StudiosInc / entrance

General purpose Display Manager built on the Enlightenment Foundation Libraries (EFL)
GNU General Public License v3.0
24 stars 10 forks source link

[MERGED] README.md changes, better issue templates, improved .gitignore and French translation #49

Closed TheTechRobo closed 3 years ago

TheTechRobo commented 4 years ago

Love what you're doing here :D

wltjr commented 3 years ago

Hi there, sorry for the delay, I have been super busy. I will get on this in a couple weeks, just after the 15th. Thank you for your interest and your contributions, both are greatly appreciated!

TheTechRobo commented 3 years ago

Hi there, sorry for the delay, I have been super busy.

No issue there. I understand.

I will get on this in a couple weeks, just after the 15th.

Sounds good!

Thank you for your interest and your contributions, both are greatly appreciated!

Any time, I just really like entrance :D

TheTechRobo commented 3 years ago

I notice it's failed, but the Shippable one seems to be something with the keys, and I can't figure out what's wrong with Travis. I hope my .gitignore modifications didn't screw something up - the issues started happening after that commit. 🤕

wltjr commented 3 years ago

I ran into that with Shippable on one of my other projects jem, https://github.com/Obsidian-StudiosInc/jem/commit/f530244eb9f99cf5b9295287f2532bc9c8011c65. People have reported it,https://github.com/Shippable/support/issues/5172, https://github.com/Shippable/support/issues/5129, not sure why its not resolved, but seems to be some what moot. I plan to switch over do a docker container at some point. I was working on that here, https://hub.docker.com/r/obsidianstudios/efl/, which is a bit outdated, I need to bump to latest EFL. That way I can test entrance and other apps in a more consistent manner, and potentially with other distros as well, as base docker images. Also, others can use the same thing for their own env, for testing, developing, etc.

Its nothing you did, but you can add the || true to the apt-get update to .travis.yml and that should resolve it for shippable.

TheTechRobo commented 3 years ago

Its nothing you did, but you can add the || true to the apt-get update to .travis.yml and that should resolve it for shippable.

OK, I just did. Status is currently pending.

UPDATE: Shippable is working; travis is still loading

wltjr commented 3 years ago

@TheTechRobo I am ready to merge this, but before I do so, could you please modify your commit messages with a prefix to match the format of existing commit messages. For example, readme modifications, precede the message with README:, the one for travis.yml would be ci:, issues would likely be issues:, the translation would be po:, gitignore gitignore: and so on. Just to further indicate what the change applies to without having to add options to the log command.

It is just a personal/project preference, just to keep log format consistent. I can do it, but it modifies the hash of each and messes with the github merge support. I finally have a workflow that allows another to make the changes, me commit them, both showing on commit log, and still using github merge, showing the commit was merged.

Thank you for the contributions and the commit message modifications, I know its an odd request :smile:

TheTechRobo commented 3 years ago

@wltjr Hm, I'm not sure how to modify earlier commit names in Git. If you can explain how, I'll be happy to do it.

Another idea: "squash and merge" - this will squash everything into one commit and you can customise the name.

wltjr commented 3 years ago

@TheTechRobo you can usually do it via a git rebase -i HEAD~10 where say 10 is the number of commits to modify. From there, you can just change each to e to edit the message. If it was the last commit, you can do git commit --amend, but that is only for the last commit. To do anything more, need to rebase.

I prefer to avoid the squash and commit. I prefer small individual commits. What you have now is great, I prefer it like that in the history, it is just the prefix for each of the commit messages. It is a personal preference, thank you for working with me there. My way is more crude, I have to force push to the repo and stuff I rather avoid.

TheTechRobo commented 3 years ago

@wltjr Done. Please review the new commit names.

TheTechRobo commented 3 years ago

@wltjr

I have to force push to the repo

I did too, or else it wouldn't have understood the pointers and everything would be screwed up. I hope you don't care about specific date and time lol because they were overwritten.

TheTechRobo commented 3 years ago

change each to e

I changed each to reword, is this OK? (Was following a guide https://linuxize.com/post/change-git-commit-message/)

wltjr commented 3 years ago

@TheTechRobo excellent, one more thing, soo sorry, can you remove the [PATCH 1/8] portions, so the messages start with the prefix you added. That is my fault for not catching when I requested the initial change. If github merge was more flexible allowing outside modifications, I could do it myself, but every time I do, it messes up the merge ability on github.

You force pushing is different than me doing it. When I force push to the main repo, it can mess up forks or PRs like yours that are base on specific hashes.

What you did was perfect thanks! I just missed the patch part, very sorry for that and having to repeat... Thanks for dealing with my preferences.

TheTechRobo commented 3 years ago

image

TheTechRobo commented 3 years ago

image

that is my screen after another rebase -i, what do I do here?

wltjr commented 3 years ago

Perfect! Thank you very much for doing that. Give me a few to wrap up a few other commits and I will merge this PR. Thank you for your contributions and the changes!

TheTechRobo commented 3 years ago

@wltjr Any time! I'm just a bit confused on what the "PATCH [1/8]" were; they don't exist on my screen.

TheTechRobo commented 3 years ago

I'll be back in about 10 minutes if you have any other feedback.

wltjr commented 3 years ago

I was able to merge, but something happened, I cannot finish it via github. It is super quirky. Seems to be something with the changes made here, not sure. It is some what moot, I just like to show the PR was merged not just closed.

TheTechRobo commented 3 years ago

That's really annoying how that happens.

TheTechRobo commented 3 years ago

ah, i think I know why : your merging had no merge commit i think, maybe that's it?

wltjr commented 3 years ago

No, it was something with changes here I believe, or maybe it was what I did. It worked a few times before with other PRs, I saved notes on it. It is not a big deal, I just have to close this vs merge. But if you look at the repo, the commits were merged.

GIthub does not provide a good way to merge commits with one person being the author and another the committer. I had worked it out, so I could commit the stuff locally, then push, and hit the merge button after, which does nothing at that point but close the PR as merged since the commits were already merged. Not sure why it did not work this time. I think I made other changes to the repo after your first change.

You can try to rebase your PR, but since I merged it, will have to do that with a local copy and not pull from master, since that stuff is already there.

It is some what moot, it just robs you of a PR contribution on your github activity is all.

TheTechRobo commented 3 years ago

@wltjr

it just robs you of a PR contribution on your github activity is all.

That doesn't matter enough for that much work. I'll close the PR.

TheTechRobo commented 3 years ago

Did you know that if you press the down arrow beside Merge Pull Request, you can click "Rebase and merge" and it will do where I am the author and you are the commiter? (Just to be safe, try it in a test repo first.)