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

Multi batteries feature #28

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Multi batteries feature #28

wants to merge 6 commits into from

Conversation

rkjdid
Copy link
Contributor

@rkjdid rkjdid commented Aug 2, 2020

No description provided.

Copy link
Contributor Author

@rkjdid rkjdid left a comment

Choose a reason for hiding this comment

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

franchement chapeau j'ai relu un peu tout j'ai vraiment pas de remarques sur le go ou le C, ça fait plaisir :)

j'arrive plus à faire tourner go generate par contre mais ça vient de moi j'essaye de voir, c pr le fichier que t'as édité à la main j'ai l'impression types_string.go. rien d'important.

le js j'avais survolé j'avais vu quelques trucs très mineurs aussi. globalement ça a l'air bien cool, jle testerais bien à l'occase :D

je vais voir pour essayer d'intégrer ça à master et faire une release dès que je prends un peu de temps.

Merci pr le taff tu gères

@@ -40,7 +40,7 @@ func init() {
}

// restore static assets & exit
if *assetsPath != "" {
/*if *assetsPath != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

un oubli ou y'a une raison?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Pour les assetsPath, les fonctions [ RestoreAssets() ] ne sont plus les même dans le assets.go généré. J'avais comparé les 2 assets.go d'avant/après, plus aucune fonction ne correspond entre les 2.
A voir s'il faut installer go-bindata version plus vieille ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ou alors la version Linux fonctionne toujours, je testerai aussi à l'occaz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok je vois, non c pas spécialement important. la version de bindata était pas frozen anyway c de ma faute

@Chatmetaleux
Copy link
Collaborator

Merci pour ton feedback ;)
y'aura un peu refactoring à faire surtout pour les noms de variables...
Le readme.md contient quelques commentaires là dessus et 1 todo

Tu vas intégrer un setting pour démarrer en mode 1batt ou 4 batt (hack 4 piles) ?

@rkjdid
Copy link
Contributor Author

rkjdid commented Aug 4, 2020

oui c'est l'idée que j'ai. après honnêtement j'ai pas le temps et je sais pas si c completement indispensable, l'alternative étant d'avoir 2 versions

c'est toujours mieux d'avoir la même codebase et de parametrer c'est certain mais voilà y'a pas foule non plus sur le projet donc c peut-être acceptable. Si on est sur une idée de continuer à faire des features etc ça va poser problème à un moment.

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.

2 participants