homer icon indicating copy to clipboard operation
homer copied to clipboard

fix: make emails configurable

Open fleboulch opened this issue 9 months ago • 4 comments

Fix #11

fleboulch avatar May 17 '24 08:05 fleboulch

The repository has no tags yet, and this is a breaking change for ManoMano. I guess we should tag it as 1.0.0 (or maybe 0.1.0, and we tag 1.0.0 when we know for a fact another company is using it. @fleboulch , are you already using it?) and document the breaking change in the release notes. I will try to find out what is our deploy process internally to determine what our deploy process for Homer is, because we need to define that variable somewhere before deploying the new release.

greg0ire avatar May 17 '24 10:05 greg0ire

The repository has no tags yet, and this is a breaking change for ManoMano. I guess we should tag it as 1.0.0 (or maybe 0.1.0, and we tag 1.0.0 when we know for a fact another company is using it. @fleboulch , are you already using it?) and document the breaking change in the release notes. I will try to find out what is our deploy process internally to determine what our deploy process for Homer is, because we need to define that variable somewhere before deploying the new release.

Hey @greg0ire ! I hope your are doing well. In order to avoid the breaking change for now, you can setup a default value for the email patterns keeping the ManoMano value. So @fleboulch will be able to use it. And then feel free to remove the default value when ManoMano will be ready internally

pvgnd avatar May 17 '24 12:05 pvgnd

Hey mate, doing well, thanks! I asked internally about the deploy process… no answer so far, so yeah, let's do as you suggest. @fleboulch, can you please implement a fallback to the legacy values?

greg0ire avatar May 17 '24 12:05 greg0ire

Thanks guys for your feedbacks! I will implement it shortly

fleboulch avatar May 17 '24 12:05 fleboulch

I added a commit to implement the fallback behaviour. Feel free to comment

fleboulch avatar May 21 '24 07:05 fleboulch

I will try to get Js specialists at ManoMano to review this, as I'm not one of them by any means.

greg0ire avatar May 21 '24 09:05 greg0ire

Thanks a lot! I'm not also a JS expert so maybe there is some parts to improve

fleboulch avatar May 21 '24 09:05 fleboulch

I'm starting to think we are still using our internal fork, as MRs keep flowing to it, so I might ask you to git reset --hard HEAD^ && git push --force at some point, just trying to get an official confirmation on this.

greg0ire avatar May 21 '24 10:05 greg0ire

@fleboulch I've been able to confirm that we are still not using the open-source version of Homer internally, which means this won't be a breaking change. You can drop that last commit with the instructions in my previous message. Sorry for wasting your time with this :sweat:

greg0ire avatar May 21 '24 14:05 greg0ire

I removed it. I'm seeing a step in red because my commit is not signed. Is it a mandatory policy?
I've never seen this policy in other open source projects but I can sign it if you really need this

fleboulch avatar May 21 '24 14:05 fleboulch

@cicoub13 do you recall why signed commits are a requirement? I'm not saying Homer isn't a sensitive project (after all, it has access to Gitlab and Slack), but I'm not sure how much sense it makes in the context of open source: it just guarantees that somebody is not trying to pass as somebody else, but since we don't particularly do a background check on people in the first place, I don't see the point.

greg0ire avatar May 21 '24 17:05 greg0ire

@cicoub13 do you recall why signed commits are a requirement?

We required that for internal/private repositories. I don't see any reason for public ones (as you said, we cannot require identity verification for contributors). I removed it for this repository ✅

cicoub13 avatar May 24 '24 08:05 cicoub13

Thanks @fleboulch !

greg0ire avatar May 27 '24 11:05 greg0ire