#129 Module to display vps info

Fusionné
jocelyn a fusionné 6 commits à partir de ARN/vps vers FFDN/master il y a 6 ans
ljf a commenté il y a 7 ans

This module allow to display vps info like IPs, vnc console or fingerprint.

It works.

TODO:

  • Documentation
  • Bug : "None" displayed to the user (if no IPs are set)
This module allow to display vps info like IPs, vnc console or fingerprint. It works. TODO: - Documentation - Bug : "None" displayed to the user (if no IPs are set)
jocelyn a commenté il y a 7 ans
Propriétaire

@ljf there are missing test data (in offers.json).

That lines in your PR refers to test data : 

self.offer = Offer.objects.filter(configuration_type="VPSConfiguration")[0]

That seem to be missing (in offers.json ?).

@ljf there are missing test data (in offers.json). That lines in your PR refers to test data :  ``` self.offer = Offer.objects.filter(configuration_type="VPSConfiguration")[0] ``` That seem to be missing (in offers.json ?).
jocelyn a commenté il y a 7 ans
Propriétaire

@ljf bump

@ljf bump
ljf a commenté il y a 6 ans
Collaborateur

I fixed unit tests

For me it's ok to be merged

I fixed unit tests For me it's ok to be merged
alexAubin a commenté il y a 6 ans
Collaborateur

Bumpitybump,

It would be nice to merge this :/

As far as I remember, Jocelyn was worried that this code should be factorized because there's a lot in common between VPS, VPN and Housing ... which I agree with. On the other hand, I doubt that we'll have manpower for this anytime soon and it's already working and is already a dependency for some other PRs if I remember correctly :s ...

What are your thoughts on this ?

Bumpitybump, It would be nice to merge this :/ As far as I remember, Jocelyn was worried that this code should be factorized because there's a lot in common between VPS, VPN and Housing ... which I agree with. On the other hand, I doubt that we'll have manpower for this anytime soon and it's already working and is already a dependency for some other PRs if I remember correctly :s ... What are your thoughts on this ?
jocelyn a commenté il y a 6 ans
Propriétaire

Fair enough. Lets not stay stuck here. I suggest that you merge and just take the time to commit big fat warning comments urging anyone starting to work on those parts to start by factorizing the code.

But I'd be verry happy if we avoid doing the same again in the future :-).

WDYT ?

Fair enough. Lets not stay stuck here. I suggest that you merge and just take the time to commit big fat warning comments urging anyone starting to work on those parts to start by factorizing the code. But I'd be verry happy if we avoid doing the same again in the future :-). WDYT ?
jocelyn a commenté il y a 6 ans
Propriétaire

Soyons fous :-)

Soyons fous :-)
Cette Pull Request a été fusionnée avec succès !
Connectez-vous pour rejoindre cette conversation.
Aucun jalon
Pas d'assignataire
3 Participants
Chargement…
Annuler
Enregistrer
Il n'existe pas encore de contenu.